protocol: canonicalize file system permissions

codex-rs/app-server-protocol/src/protocol/v2.rs

@@ -1154,19 +1154,27 @@ pub struct AdditionalFileSystemPermissions {
 
 impl From<CoreFileSystemPermissions> for AdditionalFileSystemPermissions {
     fn from(value: CoreFileSystemPermissions) -> Self {
-        Self {
-            read: value.read,
-            write: value.write,
-        }
+        let (read, write) = value.legacy_read_write_roots().unwrap_or_else(|| {
+            let read = value
+                .explicit_path_entries()
+                .filter_map(|(path, access)| access.can_read().then_some(path.clone()))
+                .collect::<Vec<_>>();
+            let write = value
+                .explicit_path_entries()
+                .filter_map(|(path, access)| access.can_write().then_some(path.clone()))
+                .collect::<Vec<_>>();
+            (
+                (!read.is_empty()).then_some(read),
+                (!write.is_empty()).then_some(write),
+            )
+        });
+        Self { read, write }
     }
 }
 
 impl From<AdditionalFileSystemPermissions> for CoreFileSystemPermissions {
     fn from(value: AdditionalFileSystemPermissions) -> Self {
-        Self {
-            read: value.read,
-            write: value.write,
-        }
+        CoreFileSystemPermissions::from_read_write_roots(value.read, value.write)
     }
 }
 
@@ -6761,16 +6769,16 @@ mod tests {
                 network: Some(CoreNetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(CoreFileSystemPermissions {
-                    read: Some(vec![
+                file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
+                    Some(vec![
                         AbsolutePathBuf::try_from(PathBuf::from(read_only_path))
                             .expect("path must be absolute"),
                     ]),
-                    write: Some(vec![
+                    Some(vec![
                         AbsolutePathBuf::try_from(PathBuf::from(read_write_path))
                             .expect("path must be absolute"),
                     ]),
-                }),
+                )),
             }
         );
     }
@@ -6854,16 +6862,16 @@ mod tests {
                 network: Some(CoreNetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(CoreFileSystemPermissions {
-                    read: Some(vec![
+                file_system: Some(CoreFileSystemPermissions::from_read_write_roots(
+                    Some(vec![
                         AbsolutePathBuf::try_from(PathBuf::from(read_only_path))
                             .expect("path must be absolute"),
                     ]),
-                    write: Some(vec![
+                    Some(vec![
                         AbsolutePathBuf::try_from(PathBuf::from(read_write_path))
                             .expect("path must be absolute"),
                     ]),
-                }),
+                )),
             }
         );
     }

codex-rs/core/src/tools/handlers/apply_patch.rs

@@ -82,10 +82,10 @@ fn write_permissions_for_paths(
         .ok()?;
 
     let permissions = (!write_paths.is_empty()).then_some(PermissionProfile {
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![]),
-            write: Some(write_paths),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![]),
+            Some(write_paths),
+        )),
         ..Default::default()
     })?;
 

codex-rs/core/src/tools/handlers/mod.rs

@@ -238,12 +238,12 @@ mod tests {
 
     fn file_system_permissions(path: &std::path::Path) -> PermissionProfile {
         PermissionProfile {
-            file_system: Some(FileSystemPermissions {
-                read: None,
-                write: Some(vec![
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                None,
+                Some(vec![
                     AbsolutePathBuf::from_absolute_path(path).expect("absolute path"),
                 ]),
-            }),
+            )),
             ..Default::default()
         }
     }

codex-rs/exec-server/src/fs_sandbox.rs

@@ -3,6 +3,9 @@ use std::collections::HashMap;
 use codex_app_server_protocol::JSONRPCErrorError;
 use codex_protocol::models::FileSystemPermissions;
 use codex_protocol::models::PermissionProfile;
+use codex_protocol::permissions::FileSystemAccessMode;
+use codex_protocol::permissions::FileSystemPath;
+use codex_protocol::permissions::FileSystemSandboxEntry;
 use codex_protocol::permissions::FileSystemSandboxPolicy;
 use codex_protocol::permissions::NetworkSandboxPolicy;
 use codex_protocol::protocol::ReadOnlyAccess;
@@ -121,16 +124,23 @@ impl FileSystemSandboxRunner {
             match additional_permissions.and_then(|permissions| permissions.file_system.clone()) {
                 Some(mut file_system) => {
                     if let Some(helper_read_root) = &helper_read_root {
-                        let read_paths = file_system.read.get_or_insert_with(Vec::new);
-                        if !read_paths.contains(helper_read_root) {
-                            read_paths.push(helper_read_root.clone());
+                        let helper_read_entry = FileSystemSandboxEntry {
+                            path: FileSystemPath::Path {
+                                path: helper_read_root.clone(),
+                            },
+                            access: FileSystemAccessMode::Read,
+                        };
+                        if !file_system.entries.contains(&helper_read_entry) {
+                            file_system.entries.push(helper_read_entry);
                         }
                     }
                     Some(file_system)
                 }
-                None => helper_read_root.map(|helper_read_root| FileSystemPermissions {
-                    read: Some(vec![helper_read_root]),
-                    write: None,
+                None => helper_read_root.map(|helper_read_root| {
+                    FileSystemPermissions::from_read_write_roots(
+                        Some(vec![helper_read_root]),
+                        /*write*/ None,
+                    )
                 }),
             };
 
@@ -373,27 +383,20 @@ mod tests {
             network: Some(NetworkPermissions {
                 enabled: Some(true),
             }),
-            file_system: Some(FileSystemPermissions {
-                read: Some(vec![]),
-                write: Some(vec![writable.clone()]),
-            }),
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![]),
+                Some(vec![writable.clone()]),
+            )),
         }));
+        let (read, write) = permissions
+            .file_system
+            .as_ref()
+            .and_then(FileSystemPermissions::legacy_read_write_roots)
+            .expect("helper permissions should stay lossless as legacy read/write roots");
 
         assert_eq!(permissions.network, None);
-        assert_eq!(
-            permissions
-                .file_system
-                .as_ref()
-                .and_then(|fs| fs.write.clone()),
-            Some(vec![writable])
-        );
-        assert_eq!(
-            permissions
-                .file_system
-                .as_ref()
-                .and_then(|fs| fs.read.clone()),
-            Some(vec![readable])
-        );
+        assert_eq!(write, Some(vec![writable]));
+        assert_eq!(read, Some(vec![readable]));
     }
 
     #[test]
@@ -415,10 +418,10 @@ mod tests {
         assert_eq!(permissions.network, None);
         assert_eq!(
             permissions.file_system,
-            Some(FileSystemPermissions {
-                read: Some(vec![readable]),
-                write: None,
-            })
+            Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![readable]),
+                None,
+            ))
         );
     }
 }

codex-rs/exec-server/tests/file_system.rs

@@ -475,10 +475,10 @@ async fn file_system_sandboxed_write_allows_additional_write_root(use_remote: bo
     let mut sandbox = read_only_sandbox(readable_dir);
     sandbox.additional_permissions = Some(PermissionProfile {
         network: None,
-        file_system: Some(FileSystemPermissions {
-            read: None,
-            write: Some(vec![absolute_path(writable_dir)]),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            None,
+            Some(vec![absolute_path(writable_dir)]),
+        )),
     });
 
     file_system

codex-rs/protocol/src/models.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::io;
 use std::path::Path;
 use std::sync::LazyLock;
 
@@ -14,6 +15,13 @@ use ts_rs::TS;
 use crate::config_types::ApprovalsReviewer;
 use crate::config_types::CollaborationMode;
 use crate::config_types::SandboxMode;
+use crate::permissions::FileSystemAccessMode;
+use crate::permissions::FileSystemPath;
+use crate::permissions::FileSystemSandboxEntry;
+use crate::permissions::FileSystemSandboxKind;
+use crate::permissions::FileSystemSandboxPolicy;
+use crate::permissions::FileSystemSpecialPath;
+use crate::permissions::NetworkSandboxPolicy;
 use crate::protocol::AskForApproval;
 use crate::protocol::COLLABORATION_MODE_CLOSE_TAG;
 use crate::protocol::COLLABORATION_MODE_OPEN_TAG;
@@ -80,15 +88,126 @@ impl SandboxPermissions {
     }
 }
 
-#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
+#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)]
 pub struct FileSystemPermissions {
-    pub read: Option<Vec<AbsolutePathBuf>>,
-    pub write: Option<Vec<AbsolutePathBuf>>,
+    pub entries: Vec<FileSystemSandboxEntry>,
 }
 
+pub type LegacyReadWriteRoots = (Option<Vec<AbsolutePathBuf>>, Option<Vec<AbsolutePathBuf>>);
+
 impl FileSystemPermissions {
     pub fn is_empty(&self) -> bool {
-        self.read.is_none() && self.write.is_none()
+        self.entries.is_empty()
+    }
+
+    pub fn from_read_write_roots(
+        read: Option<Vec<AbsolutePathBuf>>,
+        write: Option<Vec<AbsolutePathBuf>>,
+    ) -> Self {
+        let mut entries = Vec::new();
+        if let Some(read) = read {
+            entries.extend(read.into_iter().map(|path| FileSystemSandboxEntry {
+                path: FileSystemPath::Path { path },
+                access: FileSystemAccessMode::Read,
+            }));
+        }
+        if let Some(write) = write {
+            entries.extend(write.into_iter().map(|path| FileSystemSandboxEntry {
+                path: FileSystemPath::Path { path },
+                access: FileSystemAccessMode::Write,
+            }));
+        }
+        Self { entries }
+    }
+
+    pub fn explicit_path_entries(
+        &self,
+    ) -> impl Iterator<Item = (&AbsolutePathBuf, FileSystemAccessMode)> {
+        self.entries.iter().filter_map(|entry| match &entry.path {
+            FileSystemPath::Path { path } => Some((path, entry.access)),
+            FileSystemPath::GlobPattern { .. } | FileSystemPath::Special { .. } => None,
+        })
+    }
+
+    pub fn legacy_read_write_roots(&self) -> Option<LegacyReadWriteRoots> {
+        self.as_legacy_permissions()
+            .map(|legacy| (legacy.read, legacy.write))
+    }
+
+    fn as_legacy_permissions(&self) -> Option<LegacyFileSystemPermissions> {
+        let mut read = Vec::new();
+        let mut write = Vec::new();
+
+        for entry in &self.entries {
+            let FileSystemPath::Path { path } = &entry.path else {
+                return None;
+            };
+            match entry.access {
+                FileSystemAccessMode::Read => read.push(path.clone()),
+                FileSystemAccessMode::Write => write.push(path.clone()),
+                FileSystemAccessMode::None => return None,
+            }
+        }
+
+        Some(LegacyFileSystemPermissions {
+            read: (!read.is_empty()).then_some(read),
+            write: (!write.is_empty()).then_some(write),
+        })
+    }
+}
+
+#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+struct LegacyFileSystemPermissions {
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    read: Option<Vec<AbsolutePathBuf>>,
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    write: Option<Vec<AbsolutePathBuf>>,
+}
+
+#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+struct CanonicalFileSystemPermissions {
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    entries: Vec<FileSystemSandboxEntry>,
+}
+
+#[derive(Debug, Clone, Deserialize)]
+#[serde(untagged)]
+enum FileSystemPermissionsDe {
+    Canonical(CanonicalFileSystemPermissions),
+    Legacy(LegacyFileSystemPermissions),
+}
+
+impl Serialize for FileSystemPermissions {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        if let Some(legacy) = self.as_legacy_permissions() {
+            legacy.serialize(serializer)
+        } else {
+            CanonicalFileSystemPermissions {
+                entries: self.entries.clone(),
+            }
+            .serialize(serializer)
+        }
+    }
+}
+
+impl<'de> Deserialize<'de> for FileSystemPermissions {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        match FileSystemPermissionsDe::deserialize(deserializer)? {
+            FileSystemPermissionsDe::Canonical(CanonicalFileSystemPermissions { entries }) => {
+                Ok(Self { entries })
+            }
+            FileSystemPermissionsDe::Legacy(LegacyFileSystemPermissions { read, write }) => {
+                Ok(Self::from_read_write_roots(read, write))
+            }
+        }
     }
 }
 
@@ -113,6 +232,79 @@ impl PermissionProfile {
     pub fn is_empty(&self) -> bool {
         self.network.is_none() && self.file_system.is_none()
     }
+
+    pub fn from_runtime_permissions(
+        file_system_sandbox_policy: &FileSystemSandboxPolicy,
+        network_sandbox_policy: NetworkSandboxPolicy,
+    ) -> Self {
+        Self {
+            network: Some(network_sandbox_policy.into()),
+            file_system: Some(file_system_sandbox_policy.into()),
+        }
+    }
+
+    pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self {
+        Self::from_runtime_permissions(
+            &FileSystemSandboxPolicy::from_legacy_sandbox_policy(sandbox_policy, cwd),
+            NetworkSandboxPolicy::from(sandbox_policy),
+        )
+    }
+
+    pub fn file_system_sandbox_policy(&self) -> FileSystemSandboxPolicy {
+        self.file_system.as_ref().map_or_else(
+            || FileSystemSandboxPolicy::restricted(Vec::new()),
+            FileSystemSandboxPolicy::from,
+        )
+    }
+
+    pub fn network_sandbox_policy(&self) -> NetworkSandboxPolicy {
+        if self
+            .network
+            .as_ref()
+            .and_then(|network| network.enabled)
+            .unwrap_or(false)
+        {
+            NetworkSandboxPolicy::Enabled
+        } else {
+            NetworkSandboxPolicy::Restricted
+        }
+    }
+
+    pub fn to_legacy_sandbox_policy(&self, cwd: &Path) -> io::Result<SandboxPolicy> {
+        self.file_system_sandbox_policy()
+            .to_legacy_sandbox_policy(self.network_sandbox_policy(), cwd)
+    }
+}
+
+impl From<NetworkSandboxPolicy> for NetworkPermissions {
+    fn from(value: NetworkSandboxPolicy) -> Self {
+        Self {
+            enabled: Some(value.is_enabled()),
+        }
+    }
+}
+
+impl From<&FileSystemSandboxPolicy> for FileSystemPermissions {
+    fn from(value: &FileSystemSandboxPolicy) -> Self {
+        let entries = match value.kind {
+            FileSystemSandboxKind::Restricted => value.entries.clone(),
+            FileSystemSandboxKind::Unrestricted | FileSystemSandboxKind::ExternalSandbox => {
+                vec![FileSystemSandboxEntry {
+                    path: FileSystemPath::Special {
+                        value: FileSystemSpecialPath::Root,
+                    },
+                    access: FileSystemAccessMode::Write,
+                }]
+            }
+        };
+        Self { entries }
+    }
+}
+
+impl From<&FileSystemPermissions> for FileSystemSandboxPolicy {
+    fn from(value: &FileSystemPermissions) -> Self {
+        FileSystemSandboxPolicy::restricted(value.entries.clone())
+    }
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]

codex-rs/protocol/src/permissions.rs

@@ -46,6 +46,7 @@ impl NetworkSandboxPolicy {
     Debug,
     Clone,
     Copy,
+    Hash,
     PartialEq,
     Eq,
     PartialOrd,
@@ -74,7 +75,7 @@ impl FileSystemAccessMode {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
 #[serde(tag = "kind", rename_all = "snake_case")]
 #[ts(tag = "kind")]
 pub enum FileSystemSpecialPath {
@@ -117,7 +118,7 @@ impl FileSystemSpecialPath {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
 pub struct FileSystemSandboxEntry {
     pub path: FileSystemPath,
     pub access: FileSystemAccessMode,
@@ -236,7 +237,7 @@ impl ReadDenyMatcher {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
 #[serde(tag = "type", rename_all = "snake_case")]
 #[ts(tag = "type")]
 pub enum FileSystemPath {
@@ -747,13 +748,14 @@ impl FileSystemSandboxPolicy {
             FileSystemSandboxKind::Restricted => {
                 let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok();
                 let mut include_platform_defaults = false;
-                let mut has_full_disk_read_access = false;
-                let mut has_full_disk_write_access = false;
+                let has_full_disk_read_access = self.has_full_disk_read_access();
+                let has_full_disk_write_access = self.has_full_disk_write_access();
                 let mut workspace_root_writable = false;
                 let mut writable_roots = Vec::new();
                 let mut readable_roots = Vec::new();
                 let mut tmpdir_writable = false;
                 let mut slash_tmp_writable = false;
+                let mut unbridgeable_root_write = false;
 
                 for entry in &self.entries {
                     match &entry.path {
@@ -772,10 +774,15 @@ impl FileSystemSandboxPolicy {
                         FileSystemPath::Special { value } => match value {
                             FileSystemSpecialPath::Root => match entry.access {
                                 FileSystemAccessMode::None => {}
-                                FileSystemAccessMode::Read => has_full_disk_read_access = true,
+                                FileSystemAccessMode::Read => {
+                                    if !has_full_disk_read_access
+                                        && let Some(cwd) = cwd_absolute.as_ref()
+                                    {
+                                        readable_roots.push(absolute_root_path_for_cwd(cwd));
+                                    }
+                                }
                                 FileSystemAccessMode::Write => {
-                                    has_full_disk_read_access = true;
-                                    has_full_disk_write_access = true;
+                                    unbridgeable_root_write = true;
                                 }
                             },
                             FileSystemSpecialPath::Minimal => {
@@ -870,7 +877,11 @@ impl FileSystemSandboxPolicy {
                         exclude_tmpdir_env_var: !tmpdir_writable,
                         exclude_slash_tmp: !slash_tmp_writable,
                     }
-                } else if !writable_roots.is_empty() || tmpdir_writable || slash_tmp_writable {
+                } else if unbridgeable_root_write
+                    || !writable_roots.is_empty()
+                    || tmpdir_writable
+                    || slash_tmp_writable
+                {
                     return Err(io::Error::new(
                         io::ErrorKind::InvalidInput,
                         "permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly",
@@ -2079,6 +2090,11 @@ mod tests {
         assert!(
             policy.needs_direct_runtime_enforcement(NetworkSandboxPolicy::Restricted, cwd.path(),)
         );
+        assert!(
+            policy
+                .to_legacy_sandbox_policy(NetworkSandboxPolicy::Restricted, cwd.path())
+                .is_err()
+        );
     }
 
     #[test]

codex-rs/sandboxing/src/manager_tests.rs

@@ -130,10 +130,10 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() {
                     network: Some(NetworkPermissions {
                         enabled: Some(true),
                     }),
-                    file_system: Some(FileSystemPermissions {
-                        read: Some(vec![path]),
-                        write: Some(Vec::new()),
-                    }),
+                    file_system: Some(FileSystemPermissions::from_read_write_roots(
+                        Some(vec![path]),
+                        Some(Vec::new()),
+                    )),
                 }),
             },
             policy: &SandboxPolicy::ExternalSandbox {
@@ -183,10 +183,10 @@ fn transform_additional_permissions_preserves_denied_entries() {
                 cwd: cwd.clone(),
                 env: HashMap::new(),
                 additional_permissions: Some(PermissionProfile {
-                    file_system: Some(FileSystemPermissions {
-                        read: None,
-                        write: Some(vec![allowed_path.clone()]),
-                    }),
+                    file_system: Some(FileSystemPermissions::from_read_write_roots(
+                        None,
+                        Some(vec![allowed_path.clone()]),
+                    )),
                     ..Default::default()
                 }),
             },

codex-rs/sandboxing/src/policy_transforms.rs

@@ -1,7 +1,6 @@
 use codex_protocol::models::FileSystemPermissions;
 use codex_protocol::models::NetworkPermissions;
 use codex_protocol::models::PermissionProfile;
-use codex_protocol::permissions::FileSystemAccessMode;
 use codex_protocol::permissions::FileSystemPath;
 use codex_protocol::permissions::FileSystemSandboxEntry;
 use codex_protocol::permissions::FileSystemSandboxKind;
@@ -45,13 +44,29 @@ pub fn normalize_additional_permissions(
     let file_system = additional_permissions
         .file_system
         .map(|file_system| {
-            let read = file_system
-                .read
-                .map(|paths| normalize_permission_paths(paths, "file_system.read"));
-            let write = file_system
-                .write
-                .map(|paths| normalize_permission_paths(paths, "file_system.write"));
-            FileSystemPermissions { read, write }
+            let mut entries = Vec::with_capacity(file_system.entries.len());
+            for entry in file_system.entries {
+                let path = match entry.path {
+                    FileSystemPath::Path { path } => FileSystemPath::Path {
+                        path: canonicalize_preserving_symlinks(path.as_path())
+                            .ok()
+                            .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
+                            .unwrap_or(path),
+                    },
+                    FileSystemPath::GlobPattern { pattern } => {
+                        FileSystemPath::GlobPattern { pattern }
+                    }
+                    FileSystemPath::Special { value } => FileSystemPath::Special { value },
+                };
+                let normalized_entry = FileSystemSandboxEntry {
+                    path,
+                    access: entry.access,
+                };
+                if !entries.contains(&normalized_entry) {
+                    entries.push(normalized_entry);
+                }
+            }
+            FileSystemPermissions { entries }
         })
         .filter(|file_system| !file_system.is_empty());
     Ok(PermissionProfile {
@@ -89,8 +104,7 @@ pub fn merge_permission_profiles(
             };
             let file_system = match (base.file_system.as_ref(), permissions.file_system.as_ref()) {
                 (Some(base), Some(permissions)) => Some(FileSystemPermissions {
-                    read: merge_permission_paths(base.read.as_ref(), permissions.read.as_ref()),
-                    write: merge_permission_paths(base.write.as_ref(), permissions.write.as_ref()),
+                    entries: merge_permission_entries(&base.entries, &permissions.entries),
                 })
                 .filter(|file_system| !file_system.is_empty()),
                 (Some(base), None) => Some(base.clone()),
@@ -116,11 +130,12 @@ pub fn intersect_permission_profiles(
         .file_system
         .map(|requested_file_system| {
             let granted_file_system = granted.file_system.unwrap_or_default();
-            let read =
-                intersect_permission_paths(requested_file_system.read, granted_file_system.read);
-            let write =
-                intersect_permission_paths(requested_file_system.write, granted_file_system.write);
-            FileSystemPermissions { read, write }
+            let entries = requested_file_system
+                .entries
+                .into_iter()
+                .filter(|entry| granted_file_system.entries.contains(entry))
+                .collect();
+            FileSystemPermissions { entries }
         })
         .filter(|file_system| !file_system.is_empty());
     let network = match (requested.network, granted.network) {
@@ -143,67 +158,17 @@ pub fn intersect_permission_profiles(
     }
 }
 
-fn intersect_permission_paths(
-    requested: Option<Vec<AbsolutePathBuf>>,
-    granted: Option<Vec<AbsolutePathBuf>>,
-) -> Option<Vec<AbsolutePathBuf>> {
-    requested.and_then(|requested_paths| {
-        if requested_paths.is_empty() {
-            return granted.map(|_| Vec::new());
-        }
-
-        let granted_paths = granted.unwrap_or_default();
-        Some(
-            requested_paths
-                .into_iter()
-                .filter(|path| granted_paths.contains(path))
-                .collect::<Vec<_>>(),
-        )
-        .filter(|paths| !paths.is_empty())
-    })
-}
-
-fn normalize_permission_paths(
-    paths: Vec<AbsolutePathBuf>,
-    _permission_kind: &str,
-) -> Vec<AbsolutePathBuf> {
-    let mut out = Vec::with_capacity(paths.len());
-    let mut seen = HashSet::new();
-
-    for path in paths {
-        let canonicalized = canonicalize_preserving_symlinks(path.as_path())
-            .ok()
-            .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok())
-            .unwrap_or(path);
-        if seen.insert(canonicalized.clone()) {
-            out.push(canonicalized);
+fn merge_permission_entries(
+    base: &[FileSystemSandboxEntry],
+    permissions: &[FileSystemSandboxEntry],
+) -> Vec<FileSystemSandboxEntry> {
+    let mut merged = Vec::with_capacity(base.len() + permissions.len());
+    for entry in base.iter().chain(permissions.iter()) {
+        if !merged.contains(entry) {
+            merged.push(entry.clone());
         }
     }
-
-    out
-}
-
-fn merge_permission_paths(
-    base: Option<&Vec<AbsolutePathBuf>>,
-    permissions: Option<&Vec<AbsolutePathBuf>>,
-) -> Option<Vec<AbsolutePathBuf>> {
-    match (base, permissions) {
-        (Some(base), Some(permissions)) => {
-            let mut merged = Vec::with_capacity(base.len() + permissions.len());
-            let mut seen = HashSet::with_capacity(base.len() + permissions.len());
-
-            for path in base.iter().chain(permissions.iter()) {
-                if seen.insert(path.clone()) {
-                    merged.push(path.clone());
-                }
-            }
-
-            Some(merged).filter(|paths| !paths.is_empty())
-        }
-        (Some(base), None) => Some(base.clone()),
-        (None, Some(permissions)) => Some(permissions.clone()),
-        (None, None) => None,
-    }
+    merged
 }
 
 fn dedup_absolute_paths(paths: Vec<AbsolutePathBuf>) -> Vec<AbsolutePathBuf> {
@@ -225,14 +190,24 @@ fn additional_permission_roots(
             additional_permissions
                 .file_system
                 .as_ref()
-                .and_then(|file_system| file_system.read.clone())
+                .map(|file_system| {
+                    file_system
+                        .explicit_path_entries()
+                        .filter_map(|(path, access)| access.can_read().then_some(path.clone()))
+                        .collect()
+                })
                 .unwrap_or_default(),
         ),
         dedup_absolute_paths(
             additional_permissions
                 .file_system
                 .as_ref()
-                .and_then(|file_system| file_system.write.clone())
+                .map(|file_system| {
+                    file_system
+                        .explicit_path_entries()
+                        .filter_map(|(path, access)| access.can_write().then_some(path.clone()))
+                        .collect()
+                })
                 .unwrap_or_default(),
         ),
     )
@@ -240,28 +215,14 @@ fn additional_permission_roots(
 
 fn merge_file_system_policy_with_additional_permissions(
     file_system_policy: &FileSystemSandboxPolicy,
-    extra_reads: Vec<AbsolutePathBuf>,
-    extra_writes: Vec<AbsolutePathBuf>,
+    additional_permissions: &FileSystemPermissions,
 ) -> FileSystemSandboxPolicy {
     match file_system_policy.kind {
         FileSystemSandboxKind::Restricted => {
             let mut merged_policy = file_system_policy.clone();
-            for path in extra_reads {
-                let entry = FileSystemSandboxEntry {
-                    path: FileSystemPath::Path { path },
-                    access: FileSystemAccessMode::Read,
-                };
-                if !merged_policy.entries.contains(&entry) {
-                    merged_policy.entries.push(entry);
-                }
-            }
-            for path in extra_writes {
-                let entry = FileSystemSandboxEntry {
-                    path: FileSystemPath::Path { path },
-                    access: FileSystemAccessMode::Write,
-                };
-                if !merged_policy.entries.contains(&entry) {
-                    merged_policy.entries.push(entry);
+            for entry in &additional_permissions.entries {
+                if !merged_policy.entries.contains(entry) {
+                    merged_policy.entries.push(entry.clone());
                 }
             }
             merged_policy
@@ -280,14 +241,15 @@ pub fn effective_file_system_sandbox_policy(
         return file_system_policy.clone();
     };
 
-    let (extra_reads, extra_writes) = additional_permission_roots(additional_permissions);
-    if extra_reads.is_empty() && extra_writes.is_empty() {
+    let Some(file_system_permissions) = additional_permissions.file_system.as_ref() else {
+        return file_system_policy.clone();
+    };
+    if file_system_permissions.is_empty() {
         file_system_policy.clone()
     } else {
         merge_file_system_policy_with_additional_permissions(
             file_system_policy,
-            extra_reads,
-            extra_writes,
+            file_system_permissions,
         )
     }
 }

codex-rs/sandboxing/src/policy_transforms_tests.rs

@@ -106,10 +106,10 @@ fn normalize_additional_permissions_preserves_network() {
         network: Some(NetworkPermissions {
             enabled: Some(true),
         }),
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![path.clone()]),
-            write: Some(vec![path.clone()]),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![path.clone()]),
+            Some(vec![path.clone()]),
+        )),
     })
     .expect("permissions");
 
@@ -121,10 +121,10 @@ fn normalize_additional_permissions_preserves_network() {
     );
     assert_eq!(
         permissions.file_system,
-        Some(FileSystemPermissions {
-            read: Some(vec![path.clone()]),
-            write: Some(vec![path]),
-        })
+        Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![path.clone()]),
+            Some(vec![path]),
+        ))
     );
 }
 
@@ -141,23 +141,23 @@ fn normalize_additional_permissions_preserves_symlinked_write_paths() {
     let link_write_dir =
         AbsolutePathBuf::from_absolute_path(link_root.join("write")).expect("link write dir");
     let permissions = normalize_additional_permissions(PermissionProfile {
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![]),
-            write: Some(vec![link_write_dir]),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![]),
+            Some(vec![link_write_dir]),
+        )),
         ..Default::default()
     })
     .expect("permissions");
 
     assert_eq!(
         permissions.file_system,
-        Some(FileSystemPermissions {
-            read: Some(vec![]),
-            write: Some(vec![
+        Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![]),
+            Some(vec![
                 AbsolutePathBuf::from_absolute_path(link_root.join("write"))
-                    .expect("link write dir")
+                    .expect("link write dir"),
             ]),
-        })
+        ))
     );
 }
 
@@ -165,10 +165,7 @@ fn normalize_additional_permissions_preserves_symlinked_write_paths() {
 fn normalize_additional_permissions_drops_empty_nested_profiles() {
     let permissions = normalize_additional_permissions(PermissionProfile {
         network: Some(NetworkPermissions { enabled: None }),
-        file_system: Some(FileSystemPermissions {
-            read: None,
-            write: None,
-        }),
+        file_system: Some(FileSystemPermissions::default()),
     })
     .expect("permissions");
 
@@ -183,10 +180,10 @@ fn intersect_permission_profiles_preserves_explicit_empty_requested_reads() {
     )
     .expect("absolute temp dir");
     let requested = PermissionProfile {
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![]),
-            write: Some(vec![path]),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![]),
+            Some(vec![path]),
+        )),
         ..Default::default()
     };
     let granted = requested.clone();
@@ -205,10 +202,10 @@ fn intersect_permission_profiles_drops_ungranted_nonempty_path_requests() {
     )
     .expect("absolute temp dir");
     let requested = PermissionProfile {
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![path]),
-            write: None,
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![path]),
+            None,
+        )),
         ..Default::default()
     };
 
@@ -226,10 +223,10 @@ fn intersect_permission_profiles_drops_explicit_empty_reads_without_grant() {
     )
     .expect("absolute temp dir");
     let requested = PermissionProfile {
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![]),
-            write: Some(vec![path]),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![]),
+            Some(vec![path]),
+        )),
         ..Default::default()
     };
 
@@ -258,10 +255,10 @@ fn read_only_additional_permissions_can_enable_network_without_writes() {
             network: Some(NetworkPermissions {
                 enabled: Some(true),
             }),
-            file_system: Some(FileSystemPermissions {
-                read: Some(vec![path.clone()]),
-                write: Some(Vec::new()),
-            }),
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![path.clone()]),
+                Some(Vec::new()),
+            )),
         },
     );
 
@@ -292,10 +289,10 @@ fn external_sandbox_additional_permissions_can_enable_network() {
             network: Some(NetworkPermissions {
                 enabled: Some(true),
             }),
-            file_system: Some(FileSystemPermissions {
-                read: Some(vec![path]),
-                write: Some(Vec::new()),
-            }),
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![path]),
+                Some(Vec::new()),
+            )),
         },
     );
 
@@ -331,8 +328,10 @@ fn merge_file_system_policy_with_additional_permissions_preserves_unreadable_roo
                 access: FileSystemAccessMode::None,
             },
         ]),
-        vec![allowed_path.clone()],
-        Vec::new(),
+        &FileSystemPermissions::from_read_write_roots(
+            Some(vec![allowed_path.clone()]),
+            Some(Vec::new()),
+        ),
     );
 
     assert_eq!(
@@ -402,10 +401,10 @@ fn effective_file_system_sandbox_policy_merges_additional_write_roots() {
         },
     ]);
     let additional_permissions = PermissionProfile {
-        file_system: Some(FileSystemPermissions {
-            read: Some(vec![]),
-            write: Some(vec![allowed_path.clone()]),
-        }),
+        file_system: Some(FileSystemPermissions::from_read_write_roots(
+            Some(vec![]),
+            Some(vec![allowed_path.clone()]),
+        )),
         ..Default::default()
     };
 

codex-rs/tui/src/app.rs

@@ -9037,10 +9037,10 @@ guardian_approval = true
                 network: Some(NetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![test_absolute_path("/tmp/read-only")]),
-                    write: Some(vec![test_absolute_path("/tmp/write")]),
-                }),
+                file_system: Some(FileSystemPermissions::from_read_write_roots(
+                    Some(vec![test_absolute_path("/tmp/read-only")]),
+                    Some(vec![test_absolute_path("/tmp/write")]),
+                )),
             })
         );
         assert_eq!(
@@ -9133,10 +9133,10 @@ guardian_approval = true
                 network: Some(NetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![test_absolute_path("/tmp/read-only")]),
-                    write: Some(vec![test_absolute_path("/tmp/write")]),
-                }),
+                file_system: Some(FileSystemPermissions::from_read_write_roots(
+                    Some(vec![test_absolute_path("/tmp/read-only")]),
+                    Some(vec![test_absolute_path("/tmp/write")]),
+                )),
             }
         );
     }

codex-rs/tui/src/app/app_server_requests.rs

@@ -536,10 +536,10 @@ mod tests {
                         network: Some(NetworkPermissions {
                             enabled: Some(true),
                         }),
-                        file_system: Some(FileSystemPermissions {
-                            read: Some(vec![absolute_path(read_path)]),
-                            write: Some(vec![absolute_path(write_path)]),
-                        }),
+                        file_system: Some(FileSystemPermissions::from_read_write_roots(
+                            Some(vec![absolute_path(read_path)]),
+                            Some(vec![absolute_path(write_path)]),
+                        )),
                     },
                     scope: codex_protocol::request_permissions::PermissionGrantScope::Session,
                 },

codex-rs/tui/src/app_server_approval_conversions.rs

@@ -35,12 +35,12 @@ pub(crate) fn granted_permission_profile_from_request(
         network: value.network.map(|network| AdditionalNetworkPermissions {
             enabled: network.enabled,
         }),
-        file_system: value
-            .file_system
-            .map(|file_system| AdditionalFileSystemPermissions {
-                read: file_system.read,
-                write: file_system.write,
-            }),
+        file_system: value.file_system.map(|file_system| {
+            let (read, write) = file_system
+                .legacy_read_write_roots()
+                .unwrap_or((None, None));
+            AdditionalFileSystemPermissions { read, write }
+        }),
     }
 }
 
@@ -82,10 +82,10 @@ mod tests {
                 network: Some(NetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![absolute_path("/tmp/read-only")]),
-                    write: Some(vec![absolute_path("/tmp/write")]),
-                }),
+                file_system: Some(FileSystemPermissions::from_read_write_roots(
+                    Some(vec![absolute_path("/tmp/read-only")]),
+                    Some(vec![absolute_path("/tmp/write")]),
+                )),
             }),
             codex_app_server_protocol::GrantedPermissionProfile {
                 network: Some(codex_app_server_protocol::AdditionalNetworkPermissions {

codex-rs/tui/src/bottom_pane/approval_overlay.rs

@@ -21,6 +21,7 @@ use codex_features::Features;
 use codex_protocol::ThreadId;
 use codex_protocol::mcp::RequestId;
 use codex_protocol::models::PermissionProfile;
+use codex_protocol::permissions::FileSystemAccessMode;
 use codex_protocol::protocol::ElicitationAction;
 use codex_protocol::protocol::FileChange;
 use codex_protocol::protocol::NetworkApprovalContext;
@@ -798,20 +799,26 @@ pub(crate) fn format_additional_permissions_rule(
         parts.push("network".to_string());
     }
     if let Some(file_system) = additional_permissions.file_system.as_ref() {
-        if let Some(read) = file_system.read.as_ref() {
-            let reads = read
-                .iter()
-                .map(|path| format!("`{}`", path.display()))
-                .collect::<Vec<_>>()
-                .join(", ");
+        let reads = file_system
+            .explicit_path_entries()
+            .filter_map(|(path, access)| {
+                matches!(access, FileSystemAccessMode::Read).then_some(path)
+            })
+            .map(|path| format!("`{}`", path.display()))
+            .collect::<Vec<_>>()
+            .join(", ");
+        if !reads.is_empty() {
             parts.push(format!("read {reads}"));
         }
-        if let Some(write) = file_system.write.as_ref() {
-            let writes = write
-                .iter()
-                .map(|path| format!("`{}`", path.display()))
-                .collect::<Vec<_>>()
-                .join(", ");
+        let writes = file_system
+            .explicit_path_entries()
+            .filter_map(|(path, access)| {
+                matches!(access, FileSystemAccessMode::Write).then_some(path)
+            })
+            .map(|path| format!("`{}`", path.display()))
+            .collect::<Vec<_>>()
+            .join(", ");
+        if !writes.is_empty() {
             parts.push(format!("write {writes}"));
         }
     }
@@ -965,10 +972,10 @@ mod tests {
                 network: Some(NetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![absolute_path("/tmp/readme.txt")]),
-                    write: Some(vec![absolute_path("/tmp/out.txt")]),
-                }),
+                file_system: Some(FileSystemPermissions::from_read_write_roots(
+                    Some(vec![absolute_path("/tmp/readme.txt")]),
+                    Some(vec![absolute_path("/tmp/out.txt")]),
+                )),
             },
         }
     }
@@ -1266,10 +1273,10 @@ mod tests {
     #[test]
     fn additional_permissions_exec_options_hide_execpolicy_amendment() {
         let additional_permissions = PermissionProfile {
-            file_system: Some(FileSystemPermissions {
-                read: Some(vec![absolute_path("/tmp/readme.txt")]),
-                write: Some(vec![absolute_path("/tmp/out.txt")]),
-            }),
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![absolute_path("/tmp/readme.txt")]),
+                Some(vec![absolute_path("/tmp/out.txt")]),
+            )),
             ..Default::default()
         };
         let options = exec_options(
@@ -1347,10 +1354,10 @@ mod tests {
                 network: Some(NetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![absolute_path("/tmp/readme.txt")]),
-                    write: Some(vec![absolute_path("/tmp/out.txt")]),
-                }),
+                file_system: Some(FileSystemPermissions::from_read_write_roots(
+                    Some(vec![absolute_path("/tmp/readme.txt")]),
+                    Some(vec![absolute_path("/tmp/out.txt")]),
+                )),
             }),
         };
 
@@ -1397,10 +1404,10 @@ mod tests {
                 network: Some(NetworkPermissions {
                     enabled: Some(true),
                 }),
-                file_system: Some(FileSystemPermissions {
-                    read: Some(vec![absolute_path("/tmp/readme.txt")]),
-                    write: Some(vec![absolute_path("/tmp/out.txt")]),
-                }),
+                file_system: Some(FileSystemPermissions::from_read_write_roots(
+                    Some(vec![absolute_path("/tmp/readme.txt")]),
+                    Some(vec![absolute_path("/tmp/out.txt")]),
+                )),
             }),
         };
 

codex-rs/tui/src/chatwidget/tests/approval_requests.rs

@@ -135,10 +135,10 @@ fn app_server_exec_approval_request_preserves_permissions_context() {
             network: Some(NetworkPermissions {
                 enabled: Some(true),
             }),
-            file_system: Some(FileSystemPermissions {
-                read: Some(vec![read_path]),
-                write: Some(vec![write_path]),
-            }),
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![read_path]),
+                Some(vec![write_path]),
+            )),
         })
     );
 }
@@ -172,10 +172,10 @@ fn app_server_request_permissions_preserves_file_system_permissions() {
             network: Some(NetworkPermissions {
                 enabled: Some(true),
             }),
-            file_system: Some(FileSystemPermissions {
-                read: Some(vec![read_path]),
-                write: Some(vec![write_path]),
-            }),
+            file_system: Some(FileSystemPermissions::from_read_write_roots(
+                Some(vec![read_path]),
+                Some(vec![write_path]),
+            )),
         }
     );
 }