Make unrestricted sandbox network access explicit

crates/acp_thread/src/terminal.rs

@@ -43,12 +43,8 @@ pub struct SandboxWrap {
     /// model-requested paths that passed a user-approval prompt. They are
     /// merged with `writable_paths` when generating the sandbox policy.
     pub extra_write_paths: Vec<PathBuf>,
-    /// Outbound network the command may reach, as a hostname allowlist. An
-    /// empty allowlist (the default) means no network. A non-empty allowlist
-    /// (or [`Allowlist::any`]) causes an in-process HTTP/HTTPS proxy to be
-    /// spawned that enforces the policy, with the sandbox confined to its
-    /// loopback port.
-    pub network: Allowlist,
+    /// Outbound network access explicitly approved for this command.
+    pub network: SandboxNetworkAccess,
     /// Allow unrestricted filesystem writes (ignores all writable paths).
     pub allow_fs_write: bool,
     /// Whether the project (and therefore this terminal) is local. The
@@ -57,11 +53,25 @@ pub struct SandboxWrap {
     pub is_local: bool,
 }
 
-impl SandboxWrap {
-    /// Whether this wrap requests any outbound network access, and therefore
-    /// needs the in-process proxy to be spawned.
-    fn wants_network(&self) -> bool {
-        !self.network.is_deny_all()
+#[derive(Clone, Debug, Default)]
+pub enum SandboxNetworkAccess {
+    /// Block all outbound network access.
+    #[default]
+    None,
+    /// Allow only hosts in this allowlist, enforced by routing HTTP/HTTPS
+    /// through an in-process proxy and confining the command to the proxy's
+    /// loopback port.
+    Restricted(Allowlist),
+    /// Allow unrestricted outbound network access.
+    All,
+}
+
+impl SandboxNetworkAccess {
+    fn restricted_allowlist(&self) -> Option<&Allowlist> {
+        match self {
+            Self::Restricted(allowlist) => Some(allowlist),
+            Self::None | Self::All => None,
+        }
     }
 }
 
@@ -77,10 +87,8 @@ pub(crate) enum NetworkPolicy {
     Denied,
     /// Egress is confined to the in-process proxy on this loopback port.
     Proxied(u16),
-    /// Network was requested but couldn't be confined to the proxy (a
-    /// non-local project, or a host with no sandbox integration). The agent
-    /// layer widens such requests to "arbitrary network access" before
-    /// prompting (see `run_terminal_tool`), so the user approved exactly this.
+    /// The command explicitly requested, and the user approved, unrestricted
+    /// outbound network access.
     Unrestricted,
 }
 
@@ -90,7 +98,9 @@ pub(crate) enum NetworkPolicy {
 /// duration of the spawned command — dropping it deletes any on-disk
 /// config the launcher reads at startup.
 ///
-/// `network_policy` is the decision resolved by [`setup_network_proxy`];
+/// `network_policy` is the decision resolved by [`setup_network_proxy`].
+/// Unrestricted network access must be requested explicitly via
+/// [`SandboxNetworkAccess::All`].
 ///
 /// On non-macOS hosts this is a no-op: the inputs pass through unchanged
 /// and the returned handle is `None`. (We don't yet have a sandbox
@@ -146,17 +156,16 @@ pub(crate) fn apply_sandbox_wrap(
     }
 }
 
-/// Spawn the in-process network proxy for a sandboxed command, if one is
-/// needed, and wire the child's environment to route through it.
+/// Spawn the in-process network proxy for a sandboxed command with restricted
+/// network access, and wire the child's environment to route through it.
 ///
 /// Returns the proxy handle (which must outlive the command) alongside the
 /// resolved [`NetworkPolicy`] the sandbox should enforce. The handle is `Some`
-/// only when a proxy was actually spawned — a local macOS project that
-/// requested network. In every other case it is `None`, and the policy is
-/// [`NetworkPolicy::Denied`] (no network requested) or
-/// [`NetworkPolicy::Unrestricted`] (network requested but unenforceable: a
-/// non-local project, where the loopback proxy can't serve a remote terminal,
-/// or a non-macOS host with no sandbox to confine the child).
+/// only when a proxy was actually spawned. Unrestricted network access skips
+/// proxy setup and resolves to [`NetworkPolicy::Unrestricted`]. Restricted
+/// network access requires a local macOS project so the sandbox can confine
+/// egress to the proxy; otherwise this rejects the command instead of widening
+/// it.
 pub(crate) fn setup_network_proxy(
     sandbox_wrap: Option<&SandboxWrap>,
     env: &mut HashMap<String, String>,
@@ -165,15 +174,19 @@ pub(crate) fn setup_network_proxy(
     let Some(sandbox_wrap) = sandbox_wrap else {
         return Ok((None, NetworkPolicy::Denied));
     };
-    if !sandbox_wrap.wants_network() {
-        return Ok((None, NetworkPolicy::Denied));
-    }
+    let Some(allowlist) = sandbox_wrap.network.restricted_allowlist() else {
+        let policy = match &sandbox_wrap.network {
+            SandboxNetworkAccess::None => NetworkPolicy::Denied,
+            SandboxNetworkAccess::All => NetworkPolicy::Unrestricted,
+            SandboxNetworkAccess::Restricted(_) => unreachable!(),
+        };
+        return Ok((None, policy));
+    };
+
     // The proxy only buys us anything when a Seatbelt sandbox confines the
-    // child to its loopback port, and only works for local projects. When we
-    // can't enforce it, fall back to unrestricted egress (already approved as
-    // such by the agent layer).
+    // child to its loopback port, and only works for local projects.
     if !cfg!(target_os = "macos") || !sandbox_wrap.is_local {
-        return Ok((None, NetworkPolicy::Unrestricted));
+        anyhow::bail!("restricted network access requested, but no enforcing proxy is available");
     }
 
     // Chain through the user's real upstream proxy if the command's environment
@@ -188,7 +201,7 @@ pub(crate) fn setup_network_proxy(
 
     let (events_tx, events_rx) = futures::channel::mpsc::unbounded();
     let handle = ProxyHandle::spawn(ProxyConfig {
-        allowlist: sandbox_wrap.network.clone(),
+        allowlist: allowlist.clone(),
         upstream,
         events: events_tx,
     })?;
@@ -559,6 +572,19 @@ pub async fn create_terminal_entity(
 mod tests {
     use super::*;
 
+    #[test]
+    fn only_restricted_network_access_uses_proxy_allowlist() {
+        assert!(SandboxNetworkAccess::None.restricted_allowlist().is_none());
+        assert!(SandboxNetworkAccess::All.restricted_allowlist().is_none());
+        assert!(
+            SandboxNetworkAccess::Restricted(Allowlist::from_patterns([
+                http_proxy::HostPattern::parse("example.com").unwrap()
+            ]))
+            .restricted_allowlist()
+            .is_some()
+        );
+    }
+
     #[test]
     fn upstream_proxy_from_child_env_uses_from_env_precedence() {
         let mut env = HashMap::default();

crates/agent/src/tools/terminal_tool.rs

@@ -105,17 +105,17 @@ pub struct SandboxedTerminalToolInput {
     /// commands that fetch or upload (installing dependencies, cloning,
     /// pushing, downloading, etc.). Each entry must be a hostname or a
     /// leading-`*.` subdomain wildcard; IP literals and other wildcards are
-    /// rejected. Only HTTP and HTTPS are proxied — use `https://` URLs rather
-    /// than `git@`/`ssh://`. Requesting hosts triggers a user approval prompt,
-    /// so only list hosts you expect the command to need.
+    /// rejected. Host-specific access is enforced by an HTTP/HTTPS proxy — use
+    /// `https://` URLs rather than `git@`/`ssh://`. Requesting hosts triggers a
+    /// user approval prompt, so only list hosts you expect the command to need.
     #[serde(default)]
     pub allow_hosts: Vec<String>,
     /// Set to `true` only if the command needs outbound network access to
     /// hosts you can't enumerate up front.
     ///
-    /// This is a broad escape hatch — prefer `allow_hosts` with specific
-    /// hostnames whenever possible, so the user knows what's being approved.
-    /// Requesting it triggers a user approval prompt.
+    /// This grants unrestricted outbound network access. Prefer `allow_hosts`
+    /// with specific hostnames whenever possible, so the user knows what's
+    /// being approved. Requesting it triggers a user approval prompt.
     #[serde(default)]
     pub allow_all_hosts: Option<bool>,
     /// Paths the command needs to write to outside the default-writable
@@ -413,7 +413,7 @@ async fn run_terminal_tool(
         Some(acp_thread::SandboxWrap {
             writable_paths,
             extra_write_paths: effective.write_paths,
-            network: network_request_to_allowlist(&effective.network),
+            network: network_request_to_sandbox_network_access(&effective.network),
             allow_fs_write: effective.allow_fs_write_all,
             is_local: is_local_project,
         })
@@ -543,13 +543,17 @@ fn join_write_paths(raw_paths: &[String], base: Option<&Path>) -> Vec<PathBuf> {
         .collect()
 }
 
-/// Convert a (validated) network request into the allowlist enforced by the
-/// in-process proxy.
-fn network_request_to_allowlist(network: &NetworkRequest) -> http_proxy::Allowlist {
+/// Convert a (validated) network request into the access mode enforced by the
+/// terminal sandbox.
+fn network_request_to_sandbox_network_access(
+    network: &NetworkRequest,
+) -> acp_thread::SandboxNetworkAccess {
     match network {
-        NetworkRequest::None => http_proxy::Allowlist::empty(),
-        NetworkRequest::AnyHost => http_proxy::Allowlist::any(),
-        NetworkRequest::Hosts(hosts) => http_proxy::Allowlist::from_patterns(hosts.iter().cloned()),
+        NetworkRequest::None => acp_thread::SandboxNetworkAccess::None,
+        NetworkRequest::AnyHost => acp_thread::SandboxNetworkAccess::All,
+        NetworkRequest::Hosts(hosts) => acp_thread::SandboxNetworkAccess::Restricted(
+            http_proxy::Allowlist::from_patterns(hosts.iter().cloned()),
+        ),
     }
 }
 
@@ -2570,6 +2574,27 @@ mod tests {
         assert!(err.contains("127.0.0.1"), "unexpected error: {err}");
     }
 
+    #[test]
+    fn test_network_request_to_sandbox_network_access_uses_explicit_unrestricted_variant() {
+        match network_request_to_sandbox_network_access(&NetworkRequest::None) {
+            acp_thread::SandboxNetworkAccess::None => {}
+            other => panic!("expected no network access, got {other:?}"),
+        }
+
+        match network_request_to_sandbox_network_access(&NetworkRequest::AnyHost) {
+            acp_thread::SandboxNetworkAccess::All => {}
+            other => panic!("expected unrestricted network access, got {other:?}"),
+        }
+
+        match network_request_to_sandbox_network_access(&host_request(&["github.com"])) {
+            acp_thread::SandboxNetworkAccess::Restricted(allowlist) => {
+                assert!(allowlist.allows("github.com"));
+                assert!(!allowlist.allows("example.com"));
+            }
+            other => panic!("expected restricted network access, got {other:?}"),
+        }
+    }
+
     #[test]
     fn test_input_schema_includes_sandbox_flags() {
         // The sandboxed terminal tool advertises these fields so the model can

crates/http_proxy/src/allowlist.rs

@@ -227,8 +227,8 @@ impl Allowlist {
         Self::default()
     }
 
-    /// An allowlist that allows any host. Useful for `allow_all_hosts: true`
-    /// approval flow — we still proxy and observe, but skip the policy check.
+    /// An allowlist that allows any host. When used with the proxy, traffic is
+    /// still observed, but no host policy check is applied.
     pub fn any() -> Self {
         Self {
             patterns: Vec::new(),

crates/http_proxy/tests/end_to_end.rs

@@ -284,8 +284,7 @@ fn ip_literal_connect_is_denied_for_pattern_allowlists() {
 
 #[test]
 fn ip_literal_connect_is_allowed_when_allowlist_allows_any() {
-    // `allow_all_hosts` matches the pre-allowlist `allow_network: true`
-    // semantics: unrestricted egress, IP literals included.
+    // `Allowlist::any()` preserves unrestricted proxy semantics, IP literals included.
     let (origin_addr, origin_join) = spawn_echo_origin(b"HTTP/1.1 200 OK\r\n\r\n");
     let (proxy, mut events) = spawn_proxy(Allowlist::any());