protocol: canonicalize file system permissions

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]),
+                /*write*/ 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(
+            /*read*/ None,
+            Some(vec![absolute_path(writable_dir)]),
+        )),
     });
 
     file_system