[codex-rs] More fine-grained sandbox flag support on Linux (#632)

##### What/Why
This PR makes it so that in Linux we actually respect the different
types of `--sandbox` flag, such that users can apply network and
filesystem restrictions in combination (currently the only supported
behavior), or just pick one or the other.

We should add similar support for OSX in a future PR.

##### Testing
From Linux devbox, updated tests to use more specific flags:
```
test linux::tests_linux::sandbox_blocks_ping ... ok
test linux::tests_linux::sandbox_blocks_getent ... ok
test linux::tests_linux::test_root_read ... ok
test linux::tests_linux::test_dev_null_write ... ok
test linux::tests_linux::sandbox_blocks_dev_tcp_redirection ... ok
test linux::tests_linux::sandbox_blocks_ssh ... ok
test linux::tests_linux::test_writable_root ... ok
test linux::tests_linux::sandbox_blocks_curl ... ok
test linux::tests_linux::sandbox_blocks_wget ... ok
test linux::tests_linux::sandbox_blocks_nc ... ok
test linux::tests_linux::test_root_write - should panic ... ok
```

##### Todo
- [ ] Add negative tests (e.g. confirm you can hit the network if you
configure filesystem only restrictions)

codex-rs/core/src/codex.rs

@@ -868,6 +868,7 @@ async fn handle_function_call(
                 sandbox_type,
                 &roots_snapshot,
                 sess.ctrl_c.clone(),
+                sess.sandbox_policy,
             )
             .await;
 
@@ -952,11 +953,14 @@ async fn handle_function_call(
 
                             let retry_roots = { sess.writable_roots.lock().unwrap().clone() };
 
+                            // This is an escalated retry; the policy will not be
+                            // examined and the sandbox has been set to `None`.
                             let retry_output_result = process_exec_tool_call(
                                 params.clone(),
                                 SandboxType::None,
                                 &retry_roots,
                                 sess.ctrl_c.clone(),
+                                sess.sandbox_policy,
                             )
                             .await;
 

codex-rs/core/src/exec.rs

@@ -15,8 +15,10 @@ use tokio::sync::Notify;
 use crate::error::CodexErr;
 use crate::error::Result;
 use crate::error::SandboxErr;
+use crate::protocol::SandboxPolicy;
 
 /// Maximum we keep for each stream (100 KiB).
+/// TODO(ragona) this should be reduced
 const MAX_STREAM_OUTPUT: usize = 100 * 1024;
 
 const DEFAULT_TIMEOUT_MS: u64 = 10_000;
@@ -55,8 +57,9 @@ async fn exec_linux(
     params: ExecParams,
     writable_roots: &[PathBuf],
     ctrl_c: Arc<Notify>,
+    sandbox_policy: SandboxPolicy,
 ) -> Result<RawExecToolCallOutput> {
-    crate::linux::exec_linux(params, writable_roots, ctrl_c).await
+    crate::linux::exec_linux(params, writable_roots, ctrl_c, sandbox_policy).await
 }
 
 #[cfg(not(target_os = "linux"))]
@@ -64,6 +67,7 @@ async fn exec_linux(
     _params: ExecParams,
     _writable_roots: &[PathBuf],
     _ctrl_c: Arc<Notify>,
+    _sandbox_policy: SandboxPolicy,
 ) -> Result<RawExecToolCallOutput> {
     Err(CodexErr::Io(io::Error::new(
         io::ErrorKind::InvalidInput,
@@ -76,6 +80,7 @@ pub async fn process_exec_tool_call(
     sandbox_type: SandboxType,
     writable_roots: &[PathBuf],
     ctrl_c: Arc<Notify>,
+    sandbox_policy: SandboxPolicy,
 ) -> Result<ExecToolCallOutput> {
     let start = Instant::now();
 
@@ -98,7 +103,9 @@ pub async fn process_exec_tool_call(
             )
             .await
         }
-        SandboxType::LinuxSeccomp => exec_linux(params, writable_roots, ctrl_c).await,
+        SandboxType::LinuxSeccomp => {
+            exec_linux(params, writable_roots, ctrl_c, sandbox_policy).await
+        }
     };
     let duration = start.elapsed();
     match raw_output_result {

codex-rs/core/src/linux.rs

@@ -9,6 +9,7 @@ use crate::error::SandboxErr;
 use crate::exec::exec;
 use crate::exec::ExecParams;
 use crate::exec::RawExecToolCallOutput;
+use crate::protocol::SandboxPolicy;
 
 use landlock::Access;
 use landlock::AccessFs;
@@ -33,6 +34,7 @@ pub async fn exec_linux(
     params: ExecParams,
     writable_roots: &[PathBuf],
     ctrl_c: Arc<Notify>,
+    sandbox_policy: SandboxPolicy,
 ) -> Result<RawExecToolCallOutput> {
     // Allow READ on /
     // Allow WRITE on /dev/null
@@ -47,34 +49,12 @@ pub async fn exec_linux(
             .expect("Failed to create runtime");
 
         rt.block_on(async {
-            let abi = ABI::V5;
-            let access_rw = AccessFs::from_all(abi);
-            let access_ro = AccessFs::from_read(abi);
-
-            let mut ruleset = Ruleset::default()
-                .set_compatibility(CompatLevel::BestEffort)
-                .handle_access(access_rw)?
-                .create()?
-                .add_rules(landlock::path_beneath_rules(&["/"], access_ro))?
-                .add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))?
-                .set_no_new_privs(true);
-
-            if !writable_roots_copy.is_empty() {
-                ruleset = ruleset.add_rules(landlock::path_beneath_rules(
-                    &writable_roots_copy,
-                    access_rw,
-                ))?;
+            if sandbox_policy.is_network_restricted() {
+                install_network_seccomp_filter_on_current_thread()?;
             }
 
-            let status = ruleset.restrict_self()?;
-
-            // TODO(wpt): Probably wanna expand this more generically and not warn every time.
-            if status.ruleset == landlock::RulesetStatus::NotEnforced {
-                return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict));
-            }
-
-            if let Err(e) = install_network_seccomp_filter() {
-                return Err(CodexErr::Sandbox(e));
+            if sandbox_policy.is_file_write_restricted() {
+                install_filesystem_landlock_rules_on_current_thread(writable_roots_copy)?;
             }
 
             exec(params, ctrl_c_copy).await
@@ -92,7 +72,33 @@ pub async fn exec_linux(
     }
 }
 
-fn install_network_seccomp_filter() -> std::result::Result<(), SandboxErr> {
+fn install_filesystem_landlock_rules_on_current_thread(writable_roots: Vec<PathBuf>) -> Result<()> {
+    let abi = ABI::V5;
+    let access_rw = AccessFs::from_all(abi);
+    let access_ro = AccessFs::from_read(abi);
+
+    let mut ruleset = Ruleset::default()
+        .set_compatibility(CompatLevel::BestEffort)
+        .handle_access(access_rw)?
+        .create()?
+        .add_rules(landlock::path_beneath_rules(&["/"], access_ro))?
+        .add_rules(landlock::path_beneath_rules(&["/dev/null"], access_rw))?
+        .set_no_new_privs(true);
+
+    if !writable_roots.is_empty() {
+        ruleset = ruleset.add_rules(landlock::path_beneath_rules(&writable_roots, access_rw))?;
+    }
+
+    let status = ruleset.restrict_self()?;
+
+    if status.ruleset == landlock::RulesetStatus::NotEnforced {
+        return Err(CodexErr::Sandbox(SandboxErr::LandlockRestrict));
+    }
+
+    Ok(())
+}
+
+fn install_network_seccomp_filter_on_current_thread() -> std::result::Result<(), SandboxErr> {
     // Build rule map.
     let mut rules: BTreeMap<i64, Vec<SeccompRule>> = BTreeMap::new();
 
@@ -156,6 +162,7 @@ mod tests_linux {
     use crate::exec::process_exec_tool_call;
     use crate::exec::ExecParams;
     use crate::exec::SandboxType;
+    use crate::protocol::SandboxPolicy;
     use std::sync::Arc;
     use tempfile::NamedTempFile;
     use tokio::sync::Notify;
@@ -172,6 +179,7 @@ mod tests_linux {
             SandboxType::LinuxSeccomp,
             writable_roots,
             Arc::new(Notify::new()),
+            SandboxPolicy::NetworkAndFileWriteRestricted,
         )
         .await
         .unwrap();
@@ -238,6 +246,7 @@ mod tests_linux {
             SandboxType::LinuxSeccomp,
             &[],
             Arc::new(Notify::new()),
+            SandboxPolicy::NetworkRestricted,
         )
         .await;
 

codex-rs/core/src/protocol.rs

@@ -100,6 +100,30 @@ pub enum SandboxPolicy {
     DangerousNoRestrictions,
 }
 
+impl SandboxPolicy {
+    pub fn is_dangerous(&self) -> bool {
+        match self {
+            SandboxPolicy::NetworkRestricted => false,
+            SandboxPolicy::FileWriteRestricted => false,
+            SandboxPolicy::NetworkAndFileWriteRestricted => false,
+            SandboxPolicy::DangerousNoRestrictions => true,
+        }
+    }
+
+    pub fn is_network_restricted(&self) -> bool {
+        matches!(
+            self,
+            SandboxPolicy::NetworkRestricted | SandboxPolicy::NetworkAndFileWriteRestricted
+        )
+    }
+
+    pub fn is_file_write_restricted(&self) -> bool {
+        matches!(
+            self,
+            SandboxPolicy::FileWriteRestricted | SandboxPolicy::NetworkAndFileWriteRestricted
+        )
+    }
+}
 /// User input
 #[non_exhaustive]
 #[derive(Debug, Clone, Deserialize, Serialize)]