Require dns+nft egress mode for OpenSandbox network policies

providers/common/ai/docs/changelog.rst

@@ -36,6 +36,14 @@ Changelog
   headroom. On Airflow < 3.3, or when ``usage_limits`` is ``None``, each attempt is checked
   and counted on its own, unchanged. See :ref:`howto/operator:llm`.
 
+.. note::
+  ``OpenSandboxBackend`` now refuses a deny-all or allowlist ``SandboxSpec``, including the
+  default ``SandboxSpec()``, unless the OpenSandbox server's egress sidecar runs in ``dns+nft``
+  mode. In the server's default ``dns`` mode the sidecar filters name resolution only, and a
+  sandbox could still connect to any numeric address. Set ``[egress] mode = "dns+nft"`` in the
+  OpenSandbox server configuration, or pass ``SandboxSpec(block_network=False)`` for a sandbox
+  that may reach the network. See :ref:`the OpenSandbox backend <sandbox-backend-opensandbox>`.
+
 0.11.0
 ......
 

providers/common/ai/docs/sandbox/backends.rst

@@ -222,10 +222,19 @@ network policy; ``allow_egress_to`` becomes explicit allow rules. With
 deployment without the egress sidecar can still run an intentionally open
 sandbox. Deny/allowlist policy requires the sidecar, so the backend reads the
 enforced policy back after creation and destroys the sandbox if it does not
-match the requested spec. ``allow_egress_to_cidrs`` is refused: OpenSandbox only
-enforces CIDR targets in ``dns+nft`` mode, and the Python SDK does not expose
-that enforcement mode on policy read-back, so this backend cannot prove the
-address-layer restriction is active.
+match the requested spec. ``allow_egress_to_cidrs`` is refused; this backend does
+not translate address ranges into OpenSandbox rules.
+
+The read-back also checks the mode the sidecar enforces the policy in, set by
+``[egress] mode`` in the server configuration. Only ``dns+nft`` drops connections at
+the address layer. In ``dns`` mode, the server's default, the sidecar filters name
+resolution and nothing else, so a deny-all sandbox could still connect to any
+numeric address, including a DNS-over-HTTPS resolver that would look up any name for
+it, and a sandbox with an allowlist could connect to addresses outside the list. The
+backend destroys the sandbox and fails the task on any mode but ``dns+nft``, so set
+``mode = "dns+nft"`` on a server that runs sandboxes with a deny-all policy or an
+allowlist, or pass ``SandboxSpec(block_network=False)`` for a sandbox that may reach
+the network.
 
 Every sandbox carries ``created-by: airflow`` metadata and an
 ``airflow-sandbox-*`` name for attribution and cleanup. The server enforces a
@@ -305,10 +314,10 @@ behaves identically everywhere:
 - **Egress allowlists.** ``sbx`` enforces ``allow_egress_to`` at the host policy
   layer; Modal matches TLS handshake names, which is weaker and has to be opted
   into; OpenSandbox enforces it in an egress sidecar, and the backend reads the
-  enforced policy back rather than trusting the create request. ``allow_egress_to_cidrs``
-  is enforced at the address layer on Modal, refused on ``sbx``, and refused by
-  OpenSandbox because its SDK cannot prove that the sidecar is running in the
-  ``dns+nft`` mode required for CIDR enforcement.
+  enforced policy and the sidecar's mode back rather than trusting the create
+  request, refusing a sidecar that filters only name resolution (``dns``) rather
+  than connections (``dns+nft``). ``allow_egress_to_cidrs`` is enforced at the
+  address layer on Modal and refused on ``sbx`` and OpenSandbox.
 - **Command timeouts.** A timeout destroys an ``sbx`` sandbox and its files;
   Modal and a server-enforced OpenSandbox timeout preserve the sandbox and files.
   OpenSandbox destroys it only if the command event stream itself stalls past the

providers/common/ai/docs/sandbox/configuration.rst

@@ -54,8 +54,13 @@ egress there is a host-level ``sbx policy``, so the backend honors the default b
 refusing to provision unless the Deployment Manager has declared
 ``host_network_policy="deny-all"``, which means a bare ``SandboxSpec()`` is refused
 under the default ``host_network_policy="unknown"`` rather than silently getting
-an open sandbox. ``allow_egress_to`` names hosts the sandbox may reach, and both
-backends refuse it until you have said something that makes it enforceable:
+an open sandbox. On OpenSandbox the default and ``allow_egress_to`` become a policy
+for the server's egress sidecar, and the backend destroys the sandbox and fails the task unless that
+sidecar runs in ``dns+nft`` mode, the one that drops connections to unlisted
+addresses rather than only name resolution (see
+:ref:`the OpenSandbox backend <sandbox-backend-opensandbox>`). ``allow_egress_to``
+names hosts the sandbox may reach, and ``sbx`` and Modal refuse it until you have
+said something that makes it enforceable:
 ``sbx`` applies it as a per-sandbox rule on top of that ``deny-all`` host policy,
 since a local rule can narrow egress and never widen it, and Modal matches
 hostnames in the TLS handshake, which is weaker than it sounds and has to be

providers/common/ai/src/airflow/providers/common/ai/sandbox/opensandbox.py

@@ -63,6 +63,8 @@
 # through ``SandboxFilter(metadata=...)``; the sbx backend uses the sandbox name
 # for the same purpose.
 _CREATED_BY_METADATA = {"created-by": "airflow"}
+# The egress sidecar's "dns" mode filters name resolution only, not connections.
+_ADDRESS_LAYER_EGRESS_MODE = "dns+nft"
 
 
 def _get_status_code(error: Exception) -> int | None:
@@ -183,6 +185,12 @@ class OpenSandboxBackend(SandboxBackend):
     :class:`~airflow.providers.common.ai.sandbox.SandboxSpec` asked for. The
     fail-closed contract is this backend's to keep, not the server's.
 
+    The check also reads the sidecar's enforcement mode. Only
+    ``[egress] mode = "dns+nft"`` in the server configuration drops connections
+    at the address layer; in ``dns`` mode the sidecar filters name resolution
+    alone and a deny-all sandbox can still connect to numeric addresses, so the
+    backend destroys the sandbox on any mode but ``dns+nft``.
+
     Command deadlines are enforced by execd. If its event stream stalls, the
     call is abandoned ``_EXEC_GRACE`` seconds past the budget, the sandbox is
     destroyed to end it, and the result reports ``timed_out`` with
@@ -285,10 +293,9 @@ def _get_network_policy(spec: SandboxSpec | None) -> NetworkPolicy | None:
             return None
         if spec.allow_egress_to_cidrs:
             raise SandboxTerminalError(
-                "SandboxSpec names allow_egress_to_cidrs, which this backend cannot safely enforce: "
-                "OpenSandbox CIDR rules require the egress sidecar's dns+nft mode, but the SDK "
-                "does not expose that enforcement mode on policy read-back. Use allow_egress_to "
-                "with hostnames or a backend with verifiable address-layer enforcement."
+                "SandboxSpec names allow_egress_to_cidrs, which this backend does not support yet. "
+                "Use allow_egress_to with hostnames, or a backend that enforces address ranges, "
+                "such as ModalSandboxBackend."
             )
         if not spec.block_network:
             if spec.allow_egress_to:
@@ -307,24 +314,74 @@ def _get_network_policy(spec: SandboxSpec | None) -> NetworkPolicy | None:
         # spellings work at runtime, but only the alias is in the typed signature.
         return NetworkPolicy(defaultAction="deny", egress=rules or None)
 
+    @staticmethod
+    def _get_egress_enforcement_mode(sandbox: SandboxSync) -> str | None:
+        """Return the mode the sandbox's egress sidecar enforces its policy in, or ``None`` if unreported."""
+        import httpx
+        from opensandbox.adapters.converter.response_handler import handle_api_error, require_parsed
+        from opensandbox.api.egress import Client
+        from opensandbox.api.egress.api.policy import get_policy
+        from opensandbox.api.egress.models import PolicyStatusResponse
+        from opensandbox.constants import DEFAULT_EGRESS_PORT
+
+        # get_egress_policy() keeps only the rules from the sidecar's /policy response and drops
+        # its enforcementMode, so read that response with the SDK's generated egress client.
+        config = sandbox.connection_config
+        endpoint = sandbox.get_endpoint(DEFAULT_EGRESS_PORT)
+        # OpenSandbox 1.1.0 has no new_owned_transport(); give this client its own SDK transport.
+        transport = config.model_copy(update={"transport": None}).with_transport_if_missing().transport
+        with Client(
+            base_url=f"{config.protocol}://{endpoint.endpoint}",
+            headers=endpoint.build_request_headers(config),
+            timeout=httpx.Timeout(config.request_timeout.total_seconds()),
+            httpx_args={"transport": transport},
+        ) as client:
+            response = get_policy.sync_detailed(client=client)
+        handle_api_error(response, "Get egress policy")
+        mode = require_parsed(response, PolicyStatusResponse, "Get egress policy").enforcement_mode
+        return mode if isinstance(mode, str) else None
+
     @staticmethod
     def _verify_network_policy(sandbox: SandboxSync, requested: NetworkPolicy) -> None:
         wanted = {rule.target for rule in requested.egress or ()}
         try:
             enforced = sandbox.get_egress_policy()
             allowed = {rule.target for rule in enforced.egress or () if rule.action == "allow"}
-            matches = enforced.default_action == "deny" and allowed == wanted
-            detail = f"enforced policy is default {enforced.default_action!r} with allow {sorted(allowed)}"
+            if enforced.default_action != "deny" or allowed != wanted:
+                problem = (
+                    f"the enforced policy is default {enforced.default_action!r} with allow "
+                    f"{sorted(allowed)}. The server may be running without its egress sidecar."
+                )
+            else:
+                mode = OpenSandboxBackend._get_egress_enforcement_mode(sandbox)
+                if mode == _ADDRESS_LAYER_EGRESS_MODE:
+                    return
+                if mode is None:
+                    problem = (
+                        "its egress sidecar does not report an enforcement mode. Upgrade the egress "
+                        "sidecar to v1.0.1 or later and configure it to run in dns+nft mode."
+                    )
+                else:
+                    problem = (
+                        f"its egress sidecar enforces it in {mode!r} mode, and only "
+                        f'"{_ADDRESS_LAYER_EGRESS_MODE}" stops connections to numeric addresses '
+                        "rather than name resolution alone. Set "
+                        f'[egress] mode = "{_ADDRESS_LAYER_EGRESS_MODE}" in the server configuration.'
+                    )
+                problem += (
+                    " Or pass SandboxSpec(block_network=False) for a sandbox that may reach the network."
+                )
         except Exception as e:
-            matches = False
-            detail = f"the policy could not be read back ({type(e).__name__})"
-        if matches:
-            return
+            code = _get_status_code(e)
+            status = f", HTTP {code}" if code is not None else ""
+            problem = (
+                f"the policy could not be read back ({type(e).__name__}{status}). The server may be "
+                "running without its egress sidecar."
+            )
         with suppress(Exception):
             sandbox.destroy()
         raise SandboxTerminalError(
-            f"OpenSandbox did not enforce the requested network policy ({detail}), so the sandbox was "
-            "destroyed. The server may be running without its egress sidecar."
+            f"OpenSandbox did not enforce the requested network policy, so the sandbox was destroyed: {problem}"
         )
 
     def create(self, *, spec: SandboxSpec | None = None) -> str:

providers/common/ai/tests/system/common/ai/README.md

@@ -64,5 +64,5 @@ pytest --system providers/common/ai/tests/system/common/ai/example_sandbox_tools
 ```
 
 The test requests the default deny-all egress policy, so the OpenSandbox server
-must have its egress sidecar configured. It also applies a 15-minute server-side
-sandbox lifetime.
+must run its egress sidecar with `[egress] mode = "dns+nft"`; the backend refuses
+a sidecar in `dns` mode. It also applies a 15-minute server-side sandbox lifetime.

providers/common/ai/tests/unit/common/ai/sandbox/test_opensandbox.py

@@ -18,17 +18,22 @@
 
 import builtins
 import io
+import re
 import threading
 import time
+from datetime import timedelta
 from types import SimpleNamespace
 from unittest import mock
 
 import pytest
 
 pytest.importorskip("opensandbox")
 
+import httpx
+from opensandbox.config import ConnectionConfigSync
+from opensandbox.constants import DEFAULT_EGRESS_PORT
 from opensandbox.exceptions import SandboxApiException
-from opensandbox.models.sandboxes import NetworkPolicy, NetworkRule
+from opensandbox.models.sandboxes import NetworkPolicy, NetworkRule, SandboxEndpoint
 
 from airflow.providers.common.ai.sandbox import base
 from airflow.providers.common.ai.sandbox.base import (
@@ -92,12 +97,18 @@ def _deny_policy(*targets: str) -> NetworkPolicy:
 
 
 def _created(policy: NetworkPolicy | Exception | None = None):
-    sandbox = mock.MagicMock(spec=["id", "get_egress_policy", "destroy"])
+    sandbox = mock.MagicMock(spec=["id", "get_egress_policy", "destroy", "connection_config", "get_endpoint"])
     sandbox.id = "created"
     if isinstance(policy, Exception):
         sandbox.get_egress_policy.side_effect = policy
     else:
         sandbox.get_egress_policy.return_value = policy
+    sandbox.connection_config = ConnectionConfigSync(
+        domain="sandbox.example", api_key="key", use_server_proxy=True, request_timeout=timedelta(seconds=12)
+    )
+    sandbox.get_endpoint.return_value = SandboxEndpoint(
+        endpoint="egress.example:18080", headers={"X-Route": "created"}
+    )
     return sandbox
 
 
@@ -207,6 +218,29 @@ def test_connection_without_host_is_terminal_rather_than_localhost(self, hook, c
 
 
 class TestCreate:
+    @pytest.fixture(autouse=True)
+    def egress_sidecar(self):
+        """Answer the egress sidecar's ``GET /policy``, reporting ``enforcement_mode`` (dns+nft unless a test sets it)."""
+        sidecar = SimpleNamespace(
+            enforcement_mode="dns+nft", status_code=200, transient_status=None, requests=[]
+        )
+
+        def send(_transport, request):
+            sidecar.requests.append(request)
+            if isinstance(sidecar.enforcement_mode, Exception):
+                raise sidecar.enforcement_mode
+            if sidecar.transient_status is not None and len(sidecar.requests) == 1:
+                return httpx.Response(sidecar.transient_status, text="retry", request=request)
+            if sidecar.status_code != 200:
+                return httpx.Response(sidecar.status_code, text="upstream failed", request=request)
+            body = {"status": "ok", "mode": "deny_all", "policy": {"defaultAction": "deny"}}
+            if sidecar.enforcement_mode is not None:
+                body["enforcementMode"] = sidecar.enforcement_mode
+            return httpx.Response(200, json=body, request=request)
+
+        with mock.patch.object(httpx.HTTPTransport, "handle_request", autospec=True, side_effect=send):
+            yield sidecar
+
     @mock.patch("opensandbox.SandboxSync.create", autospec=True)
     def test_refuses_an_owner_because_nothing_could_attach(self, create):
         # The ownership rules ride on per-sandbox metadata the attaching side reads back,
@@ -293,20 +327,86 @@ def test_open_network_with_allowlist_is_refused(self):
     def test_cidr_allowlist_is_refused_fail_closed(self):
         backend = OpenSandboxBackend()
 
-        with pytest.raises(SandboxTerminalError, match="allow_egress_to_cidrs"):
+        with pytest.raises(
+            SandboxTerminalError, match="allow_egress_to_cidrs, which this backend does not support yet"
+        ):
             backend.create(spec=SandboxSpec(allow_egress_to_cidrs=["203.0.113.0/24"]))
 
     @mock.patch("opensandbox.SandboxSync.create", autospec=True)
-    def test_enforced_deny_policy_is_read_back_before_the_sandbox_is_handed_out(self, create):
+    def test_enforced_deny_policy_is_read_back_before_the_sandbox_is_handed_out(self, create, egress_sidecar):
         create.return_value = _created(_deny_policy("pypi.org"))
         backend = OpenSandboxBackend()
         backend._connection_config = mock.sentinel.connection_config
 
         assert backend.create(spec=SandboxSpec(allow_egress_to=["pypi.org"])) == "created"
 
         create.return_value.get_egress_policy.assert_called_once_with()
+        create.return_value.get_endpoint.assert_called_once_with(DEFAULT_EGRESS_PORT)
+        [request] = egress_sidecar.requests
+        assert (request.method, str(request.url)) == ("GET", "http://egress.example:18080/policy")
+        assert request.headers["X-Route"] == "created"
+        assert request.headers["OPEN-SANDBOX-API-KEY"] == "key"
+        assert request.extensions["timeout"]["read"] == 12
         create.return_value.destroy.assert_not_called()
 
+    @pytest.mark.parametrize("transient_status", [None, 502, 503])
+    @mock.patch("opensandbox.SandboxSync.create", autospec=True)
+    def test_policy_readback_uses_owned_transport_with_proxy_environment(
+        self, create, transient_status, egress_sidecar, monkeypatch
+    ):
+        monkeypatch.setenv("HTTP_PROXY", "http://proxy.invalid:3128")
+        monkeypatch.setenv("HTTPS_PROXY", "http://proxy.invalid:3128")
+        monkeypatch.setenv("NO_PROXY", "")
+        egress_sidecar.transient_status = transient_status
+        sandbox = _created(_deny_policy())
+        sandbox.connection_config = sandbox.connection_config.with_transport_if_missing()
+        shared_transport = sandbox.connection_config.transport
+        create.return_value = sandbox
+        backend = OpenSandboxBackend()
+        backend._connection_config = mock.sentinel.connection_config
+        transport_init = httpx.HTTPTransport.__init__
+        with (
+            mock.patch.object(
+                httpx.HTTPTransport, "__init__", autospec=True, side_effect=transport_init
+            ) as init,
+            mock.patch.object(
+                type(shared_transport), "close", autospec=True, side_effect=type(shared_transport).close
+            ) as close,
+        ):
+            assert backend.create(spec=SandboxSpec()) == "created"
+        assert len(egress_sidecar.requests) == (1 if transient_status is None else 2)
+        assert all(call.kwargs.get("proxy") is None for call in init.call_args_list)
+        close.assert_called_once()
+        assert close.call_args.args[0] is not shared_transport
+        shared_transport.close()
+        sandbox.destroy.assert_not_called()
+
+    @pytest.mark.parametrize(
+        ("sidecar", "reason"),
+        [
+            ({"enforcement_mode": "dns"}, r"enforces it in 'dns' mode.*\[egress\] mode = \"dns\+nft\""),
+            ({"enforcement_mode": None}, "does not report an enforcement mode.*Upgrade.*v1.0.1 or later"),
+            ({"enforcement_mode": httpx.ConnectError("refused")}, r"could not be read back \(ConnectError\)"),
+            ({"status_code": 500}, r"could not be read back \(SandboxApiException, HTTP 500\)"),
+        ],
+        ids=["dns-only", "unreported", "unreachable-sidecar", "sidecar-http-error"],
+    )
+    @mock.patch("opensandbox.SandboxSync.create", autospec=True)
+    def test_policy_not_enforced_at_the_address_layer_destroys_the_sandbox_and_is_terminal(
+        self, create, sidecar, reason, egress_sidecar
+    ):
+        vars(egress_sidecar).update(sidecar)
+        create.return_value = _created(_deny_policy())
+        backend = OpenSandboxBackend()
+        backend._connection_config = mock.sentinel.connection_config
+
+        with pytest.raises(SandboxTerminalError, match="did not enforce the requested network policy") as err:
+            backend.create(spec=SandboxSpec())
+
+        assert re.search(reason, str(err.value))
+        create.return_value.destroy.assert_called_once_with()
+        assert backend._sandboxes == {}
+
     @pytest.mark.parametrize(
         "enforced",
         [