diff --git a/libs/python/computer-server/computer_server/cli.py b/libs/python/computer-server/computer_server/cli.py index 1d2218cd2f..fad3d70ed1 100644 --- a/libs/python/computer-server/computer_server/cli.py +++ b/libs/python/computer-server/computer_server/cli.py @@ -10,6 +10,66 @@ logger = logging.getLogger(__name__) +# Hosts that only accept connections from the local machine. +# Note: an empty/whitespace host is deliberately excluded — it binds all +# interfaces (INADDR_ANY), so it must not be treated as loopback. +_LOOPBACK_HOSTS = frozenset({"127.0.0.1", "::1", "localhost"}) + + +class InsecureBindError(ValueError): + """Raised when the server would bind a public interface with auth disabled.""" + + +def _env_truthy(name: str) -> bool: + """Return True when environment variable *name* holds a truthy value.""" + return os.environ.get(name, "").strip().lower() in ("1", "true", "yes", "y", "on") + + +def _is_loopback_host(host: str) -> bool: + """Return True when *host* only accepts connections from the local machine.""" + return host.strip().lower() in _LOOPBACK_HOSTS + + +def resolve_bind_host(requested_host: Optional[str]) -> str: + """Resolve the effective bind host, failing closed on insecure exposure. + + The server only authenticates requests when ``CONTAINER_NAME`` is set + (sandbox/cloud mode). With it unset ("local mode") every endpoint — including + ``run_command`` and file read/write — is unauthenticated, so a non-loopback + bind would expose remote command execution to the whole network. + + Resolution rules: + - No ``--host`` given: default to ``0.0.0.0`` in sandbox/cloud mode (auth is + enforced) or ``127.0.0.1`` in local mode (safe by default). + - Explicit non-loopback ``--host`` in local mode: refuse unless the operator + opts in with ``CUA_ALLOW_INSECURE``. + + Raises: + InsecureBindError: when a non-loopback host is requested in local mode + without ``CUA_ALLOW_INSECURE`` set. + """ + cloud_mode = bool(os.environ.get("CONTAINER_NAME")) + + if requested_host is None: + return "0.0.0.0" if cloud_mode else "127.0.0.1" + + if ( + not cloud_mode + and not _is_loopback_host(requested_host) + and not _env_truthy("CUA_ALLOW_INSECURE") + ): + raise InsecureBindError( + f"Refusing to bind '{requested_host}' with authentication disabled.\n" + "The computer-server is unauthenticated when CONTAINER_NAME is unset, so " + "binding a non-loopback interface would expose unauthenticated shell and " + "file access to the network. Choose one:\n" + " - Bind locally: --host 127.0.0.1 (default)\n" + " - Enable auth: set CONTAINER_NAME (sandbox/cloud mode)\n" + " - Override (unsafe): set CUA_ALLOW_INSECURE=1" + ) + + return requested_host + def parse_args(args: Optional[List[str]] = None) -> argparse.Namespace: """Parse command-line arguments.""" @@ -30,7 +90,13 @@ def parse_args(args: Optional[List[str]] = None) -> argparse.Namespace: help="Auto-detect and log the actual screen resolution at startup", ) parser.add_argument( - "--host", default="0.0.0.0", help="Host to bind the server to (default: 0.0.0.0)" + "--host", + default=None, + help=( + "Host to bind the server to. Default: 127.0.0.1 in local mode, or " + "0.0.0.0 when CONTAINER_NAME is set (authenticated sandbox/cloud mode). " + "Binding a non-loopback host in local mode requires CUA_ALLOW_INSECURE=1." + ), ) parser.add_argument( "--port", type=int, default=8000, help="Port to bind the server to (default: 8000)" @@ -109,8 +175,16 @@ def main() -> None: vnc_host = args.vnc_host or os.environ.get("CUA_VNC_HOST") logger.info(f"VNC backend enabled → {vnc_host}:{args.vnc_port}") + # Resolve the bind host, failing closed rather than exposing an + # unauthenticated server on a public interface (see issue #1892). + try: + bind_host = resolve_bind_host(args.host) + except InsecureBindError as exc: + logger.error(str(exc)) + sys.exit(1) + # Create and start the server - logger.info(f"Starting Cua Computer API server on {args.host}:{args.port}...") + logger.info(f"Starting Cua Computer API server on {bind_host}:{args.port}...") logger.info("HTTP API available at /ws, /cmd, /status endpoints") logger.info("MCP server available at /mcp endpoint (if fastmcp installed)") @@ -133,7 +207,7 @@ def main() -> None: # the module-level handler factory runs in main.py. from .server import Server - server = Server(host=args.host, port=args.port, log_level=args.log_level, **ssl_args) + server = Server(host=bind_host, port=args.port, log_level=args.log_level, **ssl_args) try: server.start() diff --git a/libs/python/computer-server/computer_server/main.py b/libs/python/computer-server/computer_server/main.py index 14689b4d4d..3edb0983ba 100644 --- a/libs/python/computer-server/computer_server/main.py +++ b/libs/python/computer-server/computer_server/main.py @@ -11,6 +11,7 @@ from contextlib import redirect_stderr, redirect_stdout from io import StringIO from typing import Any, Dict, List, Literal, Optional, Union, cast +from urllib.parse import urlsplit import aiohttp import uvicorn @@ -176,6 +177,87 @@ async def _reject_websocket(cls, receive, send, status_code): await send({"type": "websocket.close", "code": 1008}) +class CrossSiteOriginGuard: + """Reject cross-site *browser* requests to the command/control endpoints. + + Browsers attach an ``Origin`` header to ``fetch()`` and WebSocket requests; + native SDK clients (the Python/TS SDKs), ``curl``, and server-to-server + callers do not. Even with the server bound to loopback, a web page the user + is browsing runs on the same machine and could otherwise drive the server + (cross-site WebSocket hijacking / CSRF) into running shell commands or + reading files. + + This guard rejects requests whose ``Origin`` is a non-loopback site while + allowing: + - requests with no ``Origin`` header (native clients, server-to-server), and + - same-machine (loopback) origins, e.g. a locally served UI. + + Scoped to the ``/ws``, ``/cmd`` and ``/pty`` surfaces (shell, file, PTY). + ``/mcp`` and ``/status`` are intentionally left untouched. Pure ASGI (not + BaseHTTPMiddleware) so streaming/WebSocket responses are not buffered. + """ + + _DETAIL = "Cross-site origin is not allowed" + _LOOPBACK_ORIGIN_HOSTS = frozenset({"127.0.0.1", "localhost", "::1"}) + + def __init__(self, app): + self.app = app + + @classmethod + def _is_protected(cls, path: str) -> bool: + return path in ("/ws", "/cmd", "/pty") or path.startswith("/pty/") + + @classmethod + def _origin_allowed(cls, origin: Optional[str]) -> bool: + # No Origin header → not a browser cross-site request → allow. + if origin is None: + return True + host = (urlsplit(origin).hostname or "").lower() + return host in cls._LOOPBACK_ORIGIN_HOSTS + + async def __call__(self, scope, receive, send): + scope_type = scope.get("type") + if scope_type in ("http", "websocket") and self._is_protected(scope.get("path", "")): + origin: Optional[str] = None + for key, value in scope.get("headers", []): + # ASGI lowercases header names, but match case-insensitively so a + # non-normalizing server can't let a differently-cased Origin slip + # past the guard (which would be treated as "no Origin" → allowed). + if key.lower() == b"origin": + origin = value.decode("latin-1") + break + if not self._origin_allowed(origin): + if scope_type == "http": + await self._reject_http(send) + else: + await self._reject_websocket(receive, send) + return + await self.app(scope, receive, send) + + @classmethod + async def _reject_http(cls, send): + body = json.dumps({"detail": cls._DETAIL}).encode() + await send( + { + "type": "http.response.start", + "status": 403, + "headers": [ + (b"content-type", b"application/json"), + (b"content-length", str(len(body)).encode()), + ], + } + ) + await send({"type": "http.response.body", "body": body}) + + @classmethod + async def _reject_websocket(cls, receive, send): + event = await receive() + if event.get("type") != "websocket.connect": + return + # Reject the handshake outright (close before accept). 1008 = Policy Violation. + await send({"type": "websocket.close", "code": 1008}) + + app.add_middleware(UnavailableWithoutContainerMiddleware) # CORS configuration @@ -188,6 +270,10 @@ async def _reject_websocket(cls, receive, send, status_code): allow_headers=["*"], ) +# Reject cross-site browser requests to the shell/file/PTY endpoints so a web +# page the user visits cannot drive a loopback server (see issue #1892). +app.add_middleware(CrossSiteOriginGuard) + class McpBarePathRewrite: """Serve the MCP app at both /mcp and /mcp/. diff --git a/libs/python/computer-server/tests/test_secure_bind.py b/libs/python/computer-server/tests/test_secure_bind.py new file mode 100644 index 0000000000..46be20d1ff --- /dev/null +++ b/libs/python/computer-server/tests/test_secure_bind.py @@ -0,0 +1,160 @@ +"""Tests for secure-by-default networking (issue #1892). + +Covers two defenses: + +1. ``resolve_bind_host`` — the server defaults to a loopback bind in local + (unauthenticated) mode and refuses to bind a public interface unless the + operator explicitly opts in, while preserving ``0.0.0.0`` in authenticated + sandbox/cloud mode. + +2. ``CrossSiteOriginGuard`` — a pure-ASGI middleware that rejects cross-site + *browser* requests to the shell/file/PTY endpoints (cross-site WebSocket + hijacking / CSRF), while leaving native clients and ``/mcp`` untouched. +""" + +from __future__ import annotations + +import pytest + +try: + from computer_server.cli import InsecureBindError, resolve_bind_host +except ImportError as import_error: # pragma: no cover - dependency-dependent + # Only skip when the package/deps are absent. Any other error (e.g. a real + # regression inside computer_server.cli) must propagate and fail the suite. + pytest.skip( + f"computer_server.cli unavailable in this environment: {import_error}", + allow_module_level=True, + ) + + +@pytest.fixture +def clean_env(monkeypatch): + """Remove env vars that influence bind-host resolution for a clean baseline.""" + for var in ("CONTAINER_NAME", "CUA_ALLOW_INSECURE"): + monkeypatch.delenv(var, raising=False) + return monkeypatch + + +class TestResolveBindHost: + """Unit tests for the fail-closed bind-host resolution.""" + + def test_local_mode_defaults_to_loopback(self, clean_env): + # No CONTAINER_NAME → auth disabled → must not expose the network. + assert resolve_bind_host(None) == "127.0.0.1" + + def test_cloud_mode_defaults_to_all_interfaces(self, clean_env): + # CONTAINER_NAME set → auth enforced → 0.0.0.0 is the intended default. + clean_env.setenv("CONTAINER_NAME", "vm-abc") + assert resolve_bind_host(None) == "0.0.0.0" + + @pytest.mark.parametrize("host", ["127.0.0.1", "localhost", "::1"]) + def test_explicit_loopback_allowed_in_local_mode(self, clean_env, host): + assert resolve_bind_host(host) == host + + @pytest.mark.parametrize("host", ["0.0.0.0", "192.168.1.10", "::", "", " "]) + def test_explicit_public_host_refused_in_local_mode(self, clean_env, host): + # An empty/whitespace host binds all interfaces (INADDR_ANY), so it must + # be refused in local mode rather than treated as loopback. + with pytest.raises(InsecureBindError): + resolve_bind_host(host) + + def test_explicit_public_host_allowed_with_override(self, clean_env): + clean_env.setenv("CUA_ALLOW_INSECURE", "1") + assert resolve_bind_host("0.0.0.0") == "0.0.0.0" + + def test_explicit_public_host_allowed_in_cloud_mode(self, clean_env): + # Auth is enforced in cloud mode, so an explicit public bind is fine. + clean_env.setenv("CONTAINER_NAME", "vm-abc") + assert resolve_bind_host("0.0.0.0") == "0.0.0.0" + + +async def _run_guard(path, *, origin=None, scope_type="http", origin_header=b"origin"): + """Drive the guard once; return ``(inner_called, sent_messages)``.""" + try: + from computer_server.main import CrossSiteOriginGuard + except ImportError as exc: # pragma: no cover - dependency-dependent + # Skip only on missing deps; a regression inside main must fail, not skip. + pytest.skip(f"computer_server.main unavailable in this environment: {exc}") + + state = {"inner_called": False} + + async def inner(scope, receive, send): + state["inner_called"] = True + + headers = [] + if origin is not None: + headers.append((origin_header, origin.encode("latin-1"))) + scope = {"type": scope_type, "path": path, "headers": headers} + + sent = [] + + async def send(message): + sent.append(message) + + async def receive(): + return {"type": "websocket.connect"} + + await CrossSiteOriginGuard(inner)(scope, receive, send) + return state["inner_called"], sent + + +class TestCrossSiteOriginGuard: + """Standalone tests for the pure-ASGI cross-site origin guard.""" + + @pytest.mark.asyncio + async def test_no_origin_passes_through(self): + # Native SDK clients / curl send no Origin header. + called, sent = await _run_guard("/ws") + assert called is True + assert sent == [] + + @pytest.mark.asyncio + @pytest.mark.parametrize("origin", ["http://localhost:3000", "http://127.0.0.1:8080"]) + async def test_loopback_origin_passes_through(self, origin): + called, _ = await _run_guard("/cmd", origin=origin) + assert called is True + + @pytest.mark.asyncio + async def test_cross_site_http_is_rejected(self): + called, sent = await _run_guard("/cmd", origin="https://evil.example") + assert called is False + assert sent[0]["type"] == "http.response.start" + assert sent[0]["status"] == 403 + + @pytest.mark.asyncio + async def test_cross_site_websocket_is_rejected(self): + called, sent = await _run_guard( + "/ws", origin="https://evil.example", scope_type="websocket" + ) + assert called is False + assert {"type": "websocket.close", "code": 1008} in sent + + @pytest.mark.asyncio + async def test_pty_subpath_is_protected(self): + called, _ = await _run_guard( + "/pty/123/ws", origin="https://evil.example", scope_type="websocket" + ) + assert called is False + + @pytest.mark.asyncio + async def test_null_origin_is_rejected(self): + # Sandboxed iframes and file:// pages send ``Origin: null``. + called, _ = await _run_guard("/cmd", origin="null") + assert called is False + + @pytest.mark.asyncio + @pytest.mark.parametrize("origin_header", [b"Origin", b"ORIGIN"]) + async def test_cross_site_origin_detected_regardless_of_header_casing(self, origin_header): + # A non-normalizing server must not let a differently-cased Origin bypass + # the guard by being treated as "no Origin". + called, _ = await _run_guard( + "/cmd", origin="https://evil.example", origin_header=origin_header + ) + assert called is False + + @pytest.mark.asyncio + @pytest.mark.parametrize("path", ["/status", "/mcp", "/mcp/", "/responses"]) + async def test_unprotected_paths_ignore_origin(self, path): + # /mcp and /status must keep working for cross-origin connectors. + called, _ = await _run_guard(path, origin="https://evil.example") + assert called is True