Merge 5338d44cc1 into sapling-pr-archive-bolinfest

codex-rs/protocol/src/models.rs

@@ -312,7 +312,7 @@ impl DeveloperInstructions {
 
         let (sandbox_mode, writable_roots) = match sandbox_policy {
             SandboxPolicy::DangerFullAccess => (SandboxMode::DangerFullAccess, None),
-            SandboxPolicy::ReadOnly => (SandboxMode::ReadOnly, None),
+            SandboxPolicy::ReadOnly { .. } => (SandboxMode::ReadOnly, None),
             SandboxPolicy::ExternalSandbox { .. } => (SandboxMode::DangerFullAccess, None),
             SandboxPolicy::WorkspaceWrite { .. } => {
                 let roots = sandbox_policy.get_writable_roots_with_cwd(cwd);
@@ -1223,6 +1223,7 @@ mod tests {
     fn builds_permissions_from_policy() {
         let policy = SandboxPolicy::WorkspaceWrite {
             writable_roots: vec![],
+            read_only_access: Default::default(),
             network_access: true,
             exclude_tmpdir_env_var: false,
             exclude_slash_tmp: false,

codex-rs/protocol/src/protocol.rs

@@ -4,6 +4,7 @@
 //! between user and agent.
 
 use std::collections::HashMap;
+use std::collections::HashSet;
 use std::ffi::OsStr;
 use std::fmt;
 use std::path::Path;
@@ -382,6 +383,107 @@ impl NetworkAccess {
     }
 }
 
+fn default_include_platform_defaults() -> bool {
+    true
+}
+
+/// Determines how read-only file access is granted inside a restricted
+/// sandbox.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS)]
+#[strum(serialize_all = "kebab-case")]
+#[serde(tag = "type", rename_all = "kebab-case")]
+#[ts(tag = "type")]
+pub enum ReadOnlyAccess {
+    /// Restrict reads to an explicit set of roots.
+    ///
+    /// When `include_platform_defaults` is `true`, platform defaults required
+    /// for basic execution are included in addition to `readable_roots`.
+    Restricted {
+        /// Include built-in platform read roots required for basic process
+        /// execution.
+        #[serde(default = "default_include_platform_defaults")]
+        include_platform_defaults: bool,
+        /// Additional absolute roots that should be readable.
+        #[serde(default, skip_serializing_if = "Vec::is_empty")]
+        readable_roots: Vec<AbsolutePathBuf>,
+    },
+
+    /// Allow unrestricted file reads.
+    #[default]
+    FullAccess,
+}
+
+impl ReadOnlyAccess {
+    pub fn has_full_disk_read_access(&self) -> bool {
+        matches!(self, ReadOnlyAccess::FullAccess)
+    }
+
+    /// Returns the readable roots for restricted read access.
+    ///
+    /// For [`ReadOnlyAccess::FullAccess`], returns an empty list because
+    /// callers should grant blanket read access instead.
+    pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
+        let mut roots: Vec<AbsolutePathBuf> = match self {
+            ReadOnlyAccess::FullAccess => return Vec::new(),
+            ReadOnlyAccess::Restricted {
+                include_platform_defaults,
+                readable_roots,
+            } => {
+                let mut roots = readable_roots.clone();
+                if *include_platform_defaults {
+                    #[cfg(target_os = "macos")]
+                    for platform_path in [
+                        "/bin", "/dev", "/etc", "/Library", "/private", "/sbin", "/System", "/tmp",
+                        "/usr",
+                    ] {
+                        #[allow(clippy::expect_used)]
+                        roots.push(
+                            AbsolutePathBuf::from_absolute_path(platform_path)
+                                .expect("platform defaults should be absolute"),
+                        );
+                    }
+
+                    #[cfg(target_os = "linux")]
+                    for platform_path in ["/bin", "/dev", "/etc", "/lib", "/lib64", "/tmp", "/usr"]
+                    {
+                        #[allow(clippy::expect_used)]
+                        roots.push(
+                            AbsolutePathBuf::from_absolute_path(platform_path)
+                                .expect("platform defaults should be absolute"),
+                        );
+                    }
+
+                    #[cfg(target_os = "windows")]
+                    for platform_path in [
+                        r"C:\Windows",
+                        r"C:\Program Files",
+                        r"C:\Program Files (x86)",
+                        r"C:\ProgramData",
+                    ] {
+                        #[allow(clippy::expect_used)]
+                        roots.push(
+                            AbsolutePathBuf::from_absolute_path(platform_path)
+                                .expect("platform defaults should be absolute"),
+                        );
+                    }
+
+                    match AbsolutePathBuf::from_absolute_path(cwd) {
+                        Ok(cwd_root) => roots.push(cwd_root),
+                        Err(err) => {
+                            error!("Ignoring invalid cwd {cwd:?} for sandbox readable root: {err}");
+                        }
+                    }
+                }
+                roots
+            }
+        };
+
+        let mut seen = HashSet::new();
+        roots.retain(|root| seen.insert(root.to_path_buf()));
+        roots
+    }
+}
+
 /// Determines execution restrictions for model shell commands.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Display, JsonSchema, TS)]
 #[strum(serialize_all = "kebab-case")]
@@ -391,9 +493,16 @@ pub enum SandboxPolicy {
     #[serde(rename = "danger-full-access")]
     DangerFullAccess,
 
-    /// Read-only access to the entire file-system.
+    /// Read-only access configuration.
     #[serde(rename = "read-only")]
-    ReadOnly,
+    ReadOnly {
+        /// Read access granted while running under this policy.
+        #[serde(
+            default,
+            skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access"
+        )]
+        access: ReadOnlyAccess,
+    },
 
     /// Indicates the process is already in an external sandbox. Allows full
     /// disk access while honoring the provided network setting.
@@ -413,6 +522,13 @@ pub enum SandboxPolicy {
         #[serde(default, skip_serializing_if = "Vec::is_empty")]
         writable_roots: Vec<AbsolutePathBuf>,
 
+        /// Read access granted while running under this policy.
+        #[serde(
+            default,
+            skip_serializing_if = "ReadOnlyAccess::has_full_disk_read_access"
+        )]
+        read_only_access: ReadOnlyAccess,
+
         /// When set to `true`, outbound network access is allowed. `false` by
         /// default.
         #[serde(default)]
@@ -473,7 +589,9 @@ impl FromStr for SandboxPolicy {
 impl SandboxPolicy {
     /// Returns a policy with read-only disk access and no network.
     pub fn new_read_only_policy() -> Self {
-        SandboxPolicy::ReadOnly
+        SandboxPolicy::ReadOnly {
+            access: ReadOnlyAccess::FullAccess,
+        }
     }
 
     /// Returns a policy that can read the entire disk, but can only write to
@@ -482,22 +600,29 @@ impl SandboxPolicy {
     pub fn new_workspace_write_policy() -> Self {
         SandboxPolicy::WorkspaceWrite {
             writable_roots: vec![],
+            read_only_access: ReadOnlyAccess::FullAccess,
             network_access: false,
             exclude_tmpdir_env_var: false,
             exclude_slash_tmp: false,
         }
     }
 
-    /// Always returns `true`; restricting read access is not supported.
     pub fn has_full_disk_read_access(&self) -> bool {
-        true
+        match self {
+            SandboxPolicy::DangerFullAccess => true,
+            SandboxPolicy::ExternalSandbox { .. } => true,
+            SandboxPolicy::ReadOnly { access } => access.has_full_disk_read_access(),
+            SandboxPolicy::WorkspaceWrite {
+                read_only_access, ..
+            } => read_only_access.has_full_disk_read_access(),
+        }
     }
 
     pub fn has_full_disk_write_access(&self) -> bool {
         match self {
             SandboxPolicy::DangerFullAccess => true,
             SandboxPolicy::ExternalSandbox { .. } => true,
-            SandboxPolicy::ReadOnly => false,
+            SandboxPolicy::ReadOnly { .. } => false,
             SandboxPolicy::WorkspaceWrite { .. } => false,
         }
     }
@@ -506,11 +631,35 @@ impl SandboxPolicy {
         match self {
             SandboxPolicy::DangerFullAccess => true,
             SandboxPolicy::ExternalSandbox { network_access } => network_access.is_enabled(),
-            SandboxPolicy::ReadOnly => false,
+            SandboxPolicy::ReadOnly { .. } => false,
             SandboxPolicy::WorkspaceWrite { network_access, .. } => *network_access,
         }
     }
 
+    /// Returns the list of readable roots (tailored to the current working
+    /// directory) when read access is restricted.
+    ///
+    /// For policies with full read access, this returns an empty list because
+    /// callers should grant blanket reads.
+    pub fn get_readable_roots_with_cwd(&self, cwd: &Path) -> Vec<AbsolutePathBuf> {
+        let mut roots = match self {
+            SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => Vec::new(),
+            SandboxPolicy::ReadOnly { access } => access.get_readable_roots_with_cwd(cwd),
+            SandboxPolicy::WorkspaceWrite {
+                writable_roots,
+                read_only_access,
+                ..
+            } => {
+                let mut roots = read_only_access.get_readable_roots_with_cwd(cwd);
+                roots.extend(writable_roots.clone());
+                roots
+            }
+        };
+        let mut seen = HashSet::new();
+        roots.retain(|root| seen.insert(root.to_path_buf()));
+        roots
+    }
+
     /// Returns the list of writable roots (tailored to the current working
     /// directory) together with subpaths that should remain read‑only under
     /// each writable root.
@@ -518,9 +667,10 @@ impl SandboxPolicy {
         match self {
             SandboxPolicy::DangerFullAccess => Vec::new(),
             SandboxPolicy::ExternalSandbox { .. } => Vec::new(),
-            SandboxPolicy::ReadOnly => Vec::new(),
+            SandboxPolicy::ReadOnly { .. } => Vec::new(),
             SandboxPolicy::WorkspaceWrite {
                 writable_roots,
+                read_only_access: _,
                 exclude_tmpdir_env_var,
                 exclude_slash_tmp,
                 network_access: _,
@@ -2705,7 +2855,7 @@ mod tests {
                 model: "codex-mini-latest".to_string(),
                 model_provider_id: "openai".to_string(),
                 approval_policy: AskForApproval::Never,
-                sandbox_policy: SandboxPolicy::ReadOnly,
+                sandbox_policy: SandboxPolicy::new_read_only_policy(),
                 cwd: PathBuf::from("/home/user/project"),
                 reasoning_effort: Some(ReasoningEffortConfig::default()),
                 history_log_id: 0,