fix(sandbox-envs): store no egress policy when no allowed hosts

Address PR-review findings on the per-environment egress policy feature.

- An empty/untouched egress editor now stores egress_policy=None instead
  of an explicit deny-all, so re-saving a network-enabled env no longer
  changes its runtime network behavior. To block all egress, use Network
  off.
- Reject credential header names with control chars / CR-LF and values
  with newlines, guarding against header injection into the sidecar.
- Remove references to the nonexistent DAIV_SANDBOX_EGRESS_PROXY_ENABLED
  setting from the error message, model comment, docs, and tests.
- Log egress-proxy-unavailable distinctly (still fail-closed) when a
  fresh session aborts, instead of the generic seed-failure message.
- Trim the CHANGELOG entry to the user-facing feature (Keep a Changelog);
  correct the "empty hosts = no outbound access" overstatement in docs.
- Drop dead code (has_egress property, blankHost label) and extract
  _empty_egress_state() and the _MASK_SENTINEL constant to remove
  duplicated literals.

Author: srtab

CHANGELOG.md

@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
-- Added per-environment network egress policy for sandbox environments. A `SandboxEnvironment` can now carry an `egress_policy` (default/intercept + per-host rules) and encrypted `egress_secrets` (named header credentials); when set, the agent provisions them onto the daiv-sandbox MITM egress proxy at session start so credentials are injected by the sidecar and never enter the sandbox container. The egress proxy is mandatory for network-enabled sessions: if the sandbox deployment has no egress proxy configured, a network-enabled run is rejected rather than falling back to unrestricted network. Note: a network-enabled environment **without** an egress policy receives the sidecar's default deny-all (no connectivity) — configure an egress policy on those environments. The egress policy is now configurable from the sandbox-environment form UI (shown only when **Network** is enabled): define allowed hosts (glob patterns), inject per-host credentials (HTTP header name + value, encrypted at rest and never rendered back), and set the `default` (deny/allow) and `intercept` (all/credentialed) modes. Fail-closed: an environment with no allowed hosts has no outbound access.
+- Added a per-environment **network egress policy** for sandbox environments, configurable from the environment form when network is enabled. Restrict outbound traffic to an allow-list of hosts (glob patterns), attach per-host HTTP credentials (header name + value, encrypted at rest and never rendered back), and choose the `default` (deny/allow) and `intercept` (all/credentialed) traffic modes. Credentials are injected by the daiv-sandbox egress proxy and never enter the sandbox container. The egress proxy is mandatory for network-enabled sessions: if the sandbox deployment has no egress proxy configured, a network-enabled run is rejected rather than falling back to unrestricted network.
 - Added `release_orphan_queued_threads` management command to recover `QUEUED` activities left behind on threads with no active sibling (rare TOCTOU loss).
 - Added per-domain auth headers for the `web_fetch` tool to the configuration UI under **Web Fetch → Per-domain auth headers**. Each row pairs a domain (exact match) with an HTTP header name and a header value (encrypted at rest). Replaces the previous env-only `AUTOMATION_WEB_FETCH_AUTH_HEADERS` setting; the new env override is `DAIV_WEB_FETCH_AUTH_HEADERS` (same JSON shape). Operators using the old name must rename it.
 - Added a "Start a run" page at `/dashboard/runs/new/` for launching new agent runs from the UI, and a "Retry" button on terminal non-webhook activities that pre-fills the form with the original prompt, repository, ref, and max-mode flag.

daiv/automation/agent/middlewares/sandbox.py

@@ -506,18 +506,21 @@ async def _session_exists(client: DAIVSandboxClient, session_id: str) -> bool:
     async def _provision_egress(client: DAIVSandboxClient, session_id: str, sb: SandboxRuntime | None) -> None:
         """Provision the sidecar egress policy for a network-enabled session that requires it.
 
-        No-op when there is no resolved sandbox runtime, the env defines no egress policy, or network
-        is off. Only a **404** (egress proxy not configured on the sandbox — no shared egress CA,
-        unambiguous and permanent) is converted to the actionable ``SandboxEgressUnavailableError``.
-        On a fresh create this 404 is effectively unreachable: a network-enabled ``start_session`` is
-        rejected up front (400) when the sandbox has no egress proxy, so the run aborts before reaching
-        here. It survives for warm reuse — if the shared CA is rotated out between the session's start
-        and a later turn's re-provision, ``configure_egress`` 404s and we still want the actionable
-        diagnosis. Every other status propagates unchanged, including a 409 — on the egress endpoint 409
-        is ambiguous ("Session is busy" transient lock contention, or "Session has no egress proxy") and
-        the project already classifies 409 as transient (``TRANSIENT_SANDBOX_STATUS``), so it must not be
-        mislabeled as a permanent "configure the proxy" diagnosis. A propagated error still fails closed:
-        the caller's setup path aborts the run.
+        No-op when there is no resolved sandbox runtime, the env defines no egress policy
+        (``sb.egress is None``), or network is off. Note the fail-closed deny-all that
+        ``row_to_override`` substitutes for an unusable stored config is a *non-None*
+        ``EgressConfigRequest`` and is deliberately provisioned here. Only a **404** (egress proxy not
+        configured on the sandbox — no shared egress CA, unambiguous and permanent) is converted to the
+        actionable ``SandboxEgressUnavailableError``. On a fresh create this 404 is effectively
+        unreachable: a network-enabled ``start_session`` is rejected up front (400) when the sandbox has
+        no egress proxy, so the run aborts before reaching here. It survives for warm reuse — if the
+        shared CA is rotated out between the session's start and a later turn's re-provision,
+        ``configure_egress`` 404s and we still want the actionable diagnosis. Every other status
+        propagates unchanged, including a 409 — on the egress endpoint 409 is ambiguous ("Session is
+        busy" transient lock contention, or "Session has no egress proxy") and the project already
+        classifies 409 as transient (``TRANSIENT_SANDBOX_STATUS``), so it must not be mislabeled as a
+        permanent "configure the proxy" diagnosis. A propagated error still fails closed: the caller's
+        setup path aborts the run.
         """
         if sb is None or sb.egress is None or not sb.network_enabled:
             return
@@ -582,8 +585,13 @@ async def abefore_agent(self, state: StateT, runtime: Runtime[RuntimeCtx]) -> di
             )
             await client.seed_session(session_id, repo_archive=repo_archive, skills_archive=skills_archive)
             await self._provision_egress(client, session_id, sb)
-        except Exception:
-            logger.exception("Failed to build or seed sandbox session %s", session_id)
+        except Exception as exc:
+            # Distinguish the actionable egress-misconfiguration from a generic seed failure so the
+            # operator-facing diagnosis survives in the logs (both still force-close + fail-closed).
+            if isinstance(exc, SandboxEgressUnavailableError):
+                logger.error("Egress proxy unavailable for sandbox session %s; aborting run (fail-closed)", session_id)
+            else:
+                logger.exception("Failed to build or seed sandbox session %s", session_id)
             try:
                 await client.close_session(session_id, force=True)
             except Exception:

daiv/core/sandbox/client.py

@@ -237,7 +237,8 @@ async def configure_egress(self, session_id: str, request: EgressConfigRequest)
         ``httpx.HTTPStatusError`` on a non-2xx; the caller decides how to react. Status meanings:
         404 = egress proxy disabled on the sandbox (permanent); 409 = either "Session is busy"
         (transient lock contention — see ``TRANSIENT_SANDBOX_STATUS``) or "Session has no egress
-        proxy"; other 5xx are transient.
+        proxy"; other 5xx are transient. The returned ``ok`` flag is informational only: the sandbox
+        signals provisioning failures via these status codes, not via ``ok=False`` on a 2xx.
         """
         payload = {
             "policy": request.policy.model_dump(mode="json"),

daiv/sandbox_envs/forms.py

@@ -2,6 +2,7 @@
 
 import json
 import logging
+import re
 import uuid
 
 from django import forms
@@ -16,6 +17,17 @@
 _MEMORY_UNITS = {"MiB": MIB, "GiB": GIB}
 _NETWORK_TO_CHOICE = {None: "default", True: "on", False: "off"}
 _CHOICE_TO_NETWORK = {"default": None, "on": True, "off": False}
+# Valid HTTP field-name token (RFC 7230 §3.2.6). Rejects whitespace, ':' and control
+# chars so a crafted header name can't smuggle a CR/LF header-injection into the sidecar.
+_HEADER_NAME_RE = re.compile(r"^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$")
+# Placeholder shown for an unchanged secret; a submitted value equal to this (or "") means
+# "keep the stored value" on edit. The JS editor and MCP read path emit the same string.
+_MASK_SENTINEL = "******"
+
+
+def _empty_egress_state() -> dict:
+    """The egress editor's no-policy initial state (deny-all, no hosts). A fresh dict per call."""
+    return {"default": "deny", "intercept": "all", "hosts": []}
 
 
 class SandboxEnvironmentForm(forms.ModelForm):
@@ -119,7 +131,7 @@ def _initial_egress_json(self) -> str:
         :meth:`_preserve_unchanged_egress_secrets`)."""
         from core.encryption import DecryptionError
 
-        empty = json.dumps({"default": "deny", "intercept": "all", "hosts": []})
+        empty = json.dumps(_empty_egress_state())
         if self.instance.pk is None or self.instance.egress_policy is None:
             return empty
         policy = self.instance.egress_policy
@@ -219,7 +231,7 @@ def clean_repo_ids_json(self):
     def clean_egress_json(self):
         raw = (self.cleaned_data.get("egress_json") or "").strip()
         if not raw:
-            return {"default": "deny", "intercept": "all", "hosts": []}
+            return _empty_egress_state()
         try:
             parsed = json.loads(raw)
         except json.JSONDecodeError as err:
@@ -248,14 +260,19 @@ def clean_egress_json(self):
             if not methods:
                 methods = ["*"]
             header = (entry.get("header") or "").strip()
+            if header:
+                if not _HEADER_NAME_RE.match(header):
+                    raise forms.ValidationError(_("Invalid credential header name at index %d.") % idx)
+                value = entry.get("value", "")
+                if isinstance(value, str) and ("\r" in value or "\n" in value):
+                    raise forms.ValidationError(_("Credential value at index %d must not contain newlines.") % idx)
             hosts.append({
                 "host": host,
                 "methods": methods,
                 "header": header,
                 "value": entry.get("value", ""),
                 "secret_name": (entry.get("secret_name") or "").strip(),
                 "has_existing_value": bool(entry.get("has_existing_value")),
-                "has_credential": bool(header),
             })
         return {"default": default, "intercept": intercept, "hosts": hosts}
 
@@ -269,7 +286,7 @@ def clean(self):
             self.add_error("memory_value", _("Enter a memory value or switch back to default."))
         if cleaned.get("cpu_mode") == "custom" and cleaned.get("cpus") is None:
             self.add_error("cpus", _("Enter a CPU value or switch back to default."))
-        egress_parsed = cleaned.get("egress_json") or {"default": "deny", "intercept": "all", "hosts": []}
+        egress_parsed = cleaned.get("egress_json") or _empty_egress_state()
         cleaned["egress_policy"], cleaned["egress_secrets"] = self._build_egress(egress_parsed)
         return cleaned
 
@@ -295,20 +312,28 @@ def save(self, commit: bool = True) -> SandboxEnvironment:
         return instance
 
     @staticmethod
-    def _build_egress(parsed: dict) -> tuple[dict, dict]:
+    def _build_egress(parsed: dict) -> tuple[dict | None, dict]:
         """Translate the normalised egress editor state into the stored
         ``(egress_policy, egress_secrets)`` shapes. A host with a credential
         synthesises a named secret (``inject``); hosts without one get
-        ``inject=None``. Always returns an explicit policy (never None)."""
+        ``inject=None``.
+
+        Returns ``(None, {})`` when no allowed-host rules are defined. An egress
+        policy is meaningful only as an allow-list, so an untouched editor stores
+        no policy rather than a deny-all — which would otherwise change a
+        network-enabled env's behavior on every save. To block all outbound
+        traffic, set **Network** to off instead."""
         rules: list[dict] = []
         secrets: dict[str, dict] = {}
         for host in parsed["hosts"]:
             inject = None
-            if host["has_credential"]:
+            if host["header"]:
                 name = host["secret_name"] or f"s_{uuid.uuid4().hex}"
                 secrets[name] = {"header": host["header"], "value": host["value"]}
                 inject = name
             rules.append({"host": host["host"], "methods": host["methods"], "inject": inject})
+        if not rules:
+            return None, {}
         policy = {"default": parsed["default"], "intercept": parsed["intercept"], "rules": rules}
         return policy, secrets
 
@@ -333,7 +358,7 @@ def _preserve_unchanged_egress_secrets(instance: SandboxEnvironment, submitted:
             ) from err
         out: dict[str, dict] = {}
         for name, sec in submitted.items():
-            if sec.get("value", "") in ("", "******") and name in existing:
+            if sec.get("value", "") in ("", _MASK_SENTINEL) and name in existing:
                 out[name] = {**sec, "value": existing[name].get("value", "")}
             else:
                 out[name] = sec
@@ -364,7 +389,7 @@ def _preserve_unchanged_secrets(instance: SandboxEnvironment, submitted_rows: li
         out: list[dict] = []
         for row in submitted_rows:
             name = row.get("name")
-            if row.get("is_secret") and row.get("value") in ("", "******") and name in existing:
+            if row.get("is_secret") and row.get("value") in ("", _MASK_SENTINEL) and name in existing:
                 row = {**row, "value": existing[name]}
             out.append(row)
         return out

daiv/sandbox_envs/models.py

@@ -105,9 +105,10 @@ class SandboxEnvironment(TimeStampedModel):
     is_default = models.BooleanField(_("is default"), default=False)
 
     # Per-env egress policy provisioned to the daiv-sandbox sidecar proxy when network is enabled.
-    # ``None`` = no egress policy configured for this env. The daiv-sandbox egress proxy is mandatory
-    # for network-enabled sessions: a network-enabled env on a sandbox with no egress proxy configured
-    # (no shared egress CA) is rejected at session start — there is no raw-network fallback.
+    # ``None`` = no egress policy configured for this env; a policy is stored only when the env defines
+    # at least one allowed-host rule. The daiv-sandbox egress proxy is mandatory for network-enabled
+    # sessions: a network-enabled env on a sandbox with no egress proxy configured (no shared egress CA)
+    # is rejected at session start — there is no raw-network fallback.
     # NOTE: when the proxy IS configured, a network-enabled env with egress_policy=None gets the
     # sidecar's default deny-all (no connectivity) — configure a policy to grant connectivity.
     egress_policy = models.JSONField(_("egress policy"), null=True, blank=True, default=None)
@@ -194,11 +195,6 @@ def is_global_default(self) -> bool:
         """
         return self.scope == Scope.GLOBAL and bool(self.is_default)
 
-    @property
-    def has_egress(self) -> bool:
-        """True iff this env carries an egress policy (``egress_policy is not None``)."""
-        return self.egress_policy is not None
-
     def can_delete(self) -> tuple[bool, str | None]:
         """Row-level invariant gate for delete. ``promote_as_default`` must run
         first if this is the global default."""

daiv/sandbox_envs/static/sandbox_envs/js/egress-editor.js

@@ -22,7 +22,6 @@ function egressEditorState(initial = {}, labels = {}) {
         default: initial.default === "allow" ? "allow" : "deny",
         intercept: initial.intercept === "credentialed" ? "credentialed" : "all",
         labels: {
-            blankHost: labels.blankHost || "Host is required",
             duplicateHost: labels.duplicateHost || "Duplicate host",
         },
         hosts: hosts.map((h) => {

daiv/sandbox_envs/templates/sandbox_envs/_form_body.html

@@ -200,12 +200,11 @@
         {# Network access (egress) — only meaningful when network is on #}
         <fieldset x-show="effectiveNetworkOn" x-cloak class="mt-5"
                   x-data="egressEditor({{ form.egress_json.value|default:form.egress_json.initial }}, {
-                      blankHost: '{% translate 'Host is required'|escapejs %}',
                       duplicateHost: '{% translate 'Duplicate host'|escapejs %}',
                   })">
             <legend class="text-[15px] font-medium text-gray-300">{% translate "Network access" %}</legend>
             <p class="mt-1 text-sm text-gray-400">
-                {% translate "Controls which hosts the sandbox can reach. Applies only when network is enabled; with no allowed hosts the sandbox can reach nothing." %}
+                {% translate "Restricts which hosts the sandbox can reach. With Default set to deny, only the listed hosts are reachable. Leave the list empty to apply no egress policy; to block all outbound access, set Network to off." %}
             </p>
 
             <div class="mt-3 flex flex-wrap gap-6">

docs/features/sandbox-environments.md

@@ -60,7 +60,7 @@ Environment-variable values are **Fernet-encrypted before they are stored** and
 When **Network** is enabled, the **Egress** section appears at the bottom of the form. It lets you define exactly which outbound connections the sandbox container may make, with optional per-host credentials injected by the MITM proxy — credentials never enter the container.
 
 !!! info "Requires the egress proxy"
-    Egress policy is provisioned by the daiv-sandbox MITM egress proxy (`DAIV_SANDBOX_EGRESS_PROXY_ENABLED`). If the sandbox does not have the proxy enabled, starting a run that uses an egress-configured environment will **abort** (fail-closed) rather than fall back to unrestricted network.
+    Egress policy is provisioned by the daiv-sandbox MITM egress proxy. If the sandbox does not have the egress proxy enabled, starting a run that uses an egress-configured environment will **abort** (fail-closed) rather than fall back to unrestricted network.
 
 #### Allowed hosts
 
@@ -72,7 +72,7 @@ Each row in the **Allowed hosts** table defines one outbound rule:
 | **Methods** | HTTP methods this rule applies to (leave empty to match all methods) |
 | **Credential** | Optional: an HTTP header name and value injected by the proxy for every request to this host (see below) |
 
-**Fail-closed:** if the allowed hosts list is empty, no outbound requests are permitted at all (equivalent to network off, even though networking is technically enabled).
+An egress policy takes effect only when you add at least one allowed-host rule. With **Default** set to `deny`, only the listed hosts are reachable and every other outbound request is blocked. Leaving the allowed-hosts list empty stores **no** egress policy on the environment (the section is treated as untouched) — it does not block traffic. To disable networking entirely, set **Network** to **Off**.
 
 #### Per-host credentials (encrypted at rest)
 

tests/unit_tests/core/sandbox/test_schemas.py

@@ -99,6 +99,18 @@ def test_egress_config_request_rejects_dangling_inject():
         EgressConfigRequest(policy=EgressPolicy(rules=[EgressRule(host="x", inject="missing")]), secrets={})
 
 
+def test_empty_egress_config_request_is_deny_all():
+    """The fail-closed fallback in ``row_to_override`` substitutes a bare ``EgressConfigRequest()``
+    for an unusable stored config. That is only safe while the empty request denies everything — a
+    future flip of ``EgressPolicy.default`` to ``allow`` would silently turn the fallback fail-open."""
+    from core.sandbox.schemas import EgressConfigRequest
+
+    req = EgressConfigRequest()
+    assert req.policy.default == "deny"
+    assert req.policy.rules == []
+    assert req.secrets == {}
+
+
 def test_egress_secret_value_is_redacted_in_repr():
     from pydantic import SecretStr