From d965c8955ed5898d8ec1b682abd1d2528f32b1c9 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 2 Jun 2026 07:58:18 -0700 Subject: [PATCH] core: merge built-in permission profiles as TOML --- codex-rs/core/src/config/config_tests.rs | 44 ++++++- codex-rs/core/src/config/permissions.rs | 154 ++++++++++++++++++----- codex-rs/protocol/src/models.rs | 11 +- codex-rs/protocol/src/permissions.rs | 24 ++-- 4 files changed, 182 insertions(+), 51 deletions(-) diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 05cbb9828f..65d20d94f0 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -2357,7 +2357,13 @@ async fn default_permissions_profile_can_extend_builtin_workspace() -> std::io:: description: None, extends: Some(BUILT_IN_PERMISSION_PROFILE_WORKSPACE.to_string()), workspace_roots: None, - filesystem: None, + filesystem: Some(FilesystemPermissionsToml { + glob_scan_max_depth: None, + entries: BTreeMap::from([( + ":tmpdir".to_string(), + FilesystemPermissionToml::Access(FileSystemAccessMode::Read), + )]), + }), network: Some(NetworkToml { enabled: Some(true), ..Default::default() @@ -2384,6 +2390,42 @@ async fn default_permissions_profile_can_extend_builtin_workspace() -> std::io:: !policy.can_write_path_with_cwd(&cwd.path().join(".git"), cwd.path()), "expected profile extending :workspace to keep metadata carveouts, policy: {policy:?}" ); + assert!( + policy.entries.iter().any(|entry| matches!( + entry, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::SlashTmp, + }, + access: FileSystemAccessMode::Write, + } + )), + "expected profile extending :workspace to keep inherited :slash_tmp writes, policy: {policy:?}" + ); + assert!( + policy.entries.iter().any(|entry| matches!( + entry, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Read, + } + )), + "expected child :tmpdir read entry to replace the inherited write entry, policy: {policy:?}" + ); + assert!( + !policy.entries.iter().any(|entry| matches!( + entry, + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + } + )), + "expected inherited :tmpdir write entry to be removed, policy: {policy:?}" + ); assert_eq!( config.permissions.network_sandbox_policy(), NetworkSandboxPolicy::Enabled diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index a8546858f1..432f339cd6 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::collections::BTreeMap; use std::io; use std::path::Component; use std::path::Path; @@ -196,25 +197,130 @@ pub(crate) fn resolve_permission_profile( profile_name: &str, ) -> io::Result { permissions - .resolve_profile(profile_name, extensible_builtin_parent_profile_marker) + .resolve_profile(profile_name, extensible_builtin_parent_profile) .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err.to_string())) } -/// Built-in parents provide their runtime permissions below. Resolution only -/// needs an empty profile marker so inheritance can terminate while preserving -/// the built-in parent id in `inherited_profile_names`. -fn extensible_builtin_parent_profile_marker(profile_name: &str) -> Option { - matches!( - profile_name, - BUILT_IN_READ_ONLY_PROFILE | BUILT_IN_WORKSPACE_PROFILE - ) - .then_some(PermissionProfileToml { +fn extensible_builtin_parent_profile(profile_name: &str) -> Option { + let file_system = match profile_name { + BUILT_IN_READ_ONLY_PROFILE => FileSystemSandboxPolicy::read_only(), + BUILT_IN_WORKSPACE_PROFILE => FileSystemSandboxPolicy::workspace_write( + &[], + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ), + _ => return None, + }; + Some(permission_profile_toml_from_file_system_policy(file_system)) +} + +fn permission_profile_toml_from_file_system_policy( + file_system: FileSystemSandboxPolicy, +) -> PermissionProfileToml { + let mut filesystem = FilesystemPermissionsToml { + glob_scan_max_depth: file_system.glob_scan_max_depth, + entries: BTreeMap::new(), + }; + for entry in file_system.entries { + insert_filesystem_permission_toml(&mut filesystem.entries, entry); + } + PermissionProfileToml { description: None, extends: None, workspace_roots: None, - filesystem: None, + filesystem: Some(filesystem), network: None, - }) + } +} + +fn insert_filesystem_permission_toml( + entries: &mut BTreeMap, + entry: FileSystemSandboxEntry, +) { + match entry.path { + FileSystemPath::Path { path } => { + entries.insert( + path.into_path_buf().to_string_lossy().into_owned(), + FilesystemPermissionToml::Access(entry.access), + ); + } + FileSystemPath::GlobPattern { pattern } => { + entries.insert(pattern, FilesystemPermissionToml::Access(entry.access)); + } + FileSystemPath::Special { value } => { + insert_special_filesystem_permission_toml(entries, value, entry.access); + } + } +} + +fn insert_special_filesystem_permission_toml( + entries: &mut BTreeMap, + value: FileSystemSpecialPath, + access: FileSystemAccessMode, +) { + match value { + FileSystemSpecialPath::Root => { + entries.insert( + ":root".to_string(), + FilesystemPermissionToml::Access(access), + ); + } + FileSystemSpecialPath::Minimal => { + entries.insert( + ":minimal".to_string(), + FilesystemPermissionToml::Access(access), + ); + } + FileSystemSpecialPath::ProjectRoots { subpath } => { + insert_scoped_filesystem_permission_toml( + entries, + ":workspace_roots".to_string(), + subpath.unwrap_or_else(|| PathBuf::from(".")), + access, + ); + } + FileSystemSpecialPath::Tmpdir => { + entries.insert( + ":tmpdir".to_string(), + FilesystemPermissionToml::Access(access), + ); + } + FileSystemSpecialPath::SlashTmp => { + entries.insert( + ":slash_tmp".to_string(), + FilesystemPermissionToml::Access(access), + ); + } + FileSystemSpecialPath::Unknown { path, subpath } => { + if let Some(subpath) = subpath { + insert_scoped_filesystem_permission_toml(entries, path, subpath, access); + } else { + entries.insert(path, FilesystemPermissionToml::Access(access)); + } + } + }; +} + +fn insert_scoped_filesystem_permission_toml( + entries: &mut BTreeMap, + path: String, + subpath: PathBuf, + access: FileSystemAccessMode, +) { + let permission = entries + .entry(path) + .or_insert_with(|| FilesystemPermissionToml::Scoped(BTreeMap::new())); + match permission { + FilesystemPermissionToml::Scoped(scoped_entries) => { + scoped_entries.insert(subpath.to_string_lossy().into_owned(), access); + } + FilesystemPermissionToml::Access(_) => { + *permission = FilesystemPermissionToml::Scoped(BTreeMap::from([( + subpath.to_string_lossy().into_owned(), + access, + )])); + } + } } pub(crate) fn network_proxy_config_for_profile_selection( @@ -244,25 +350,10 @@ pub(crate) fn compile_permission_profile( policy_cwd: &Path, startup_warnings: &mut Vec, ) -> io::Result<(FileSystemSandboxPolicy, NetworkSandboxPolicy)> { - let ResolvedPermissionProfileToml { - profile, - inherited_profile_names, - } = resolve_permission_profile(permissions, profile_name)?; - let base_permissions = inherited_profile_names.iter().find_map(|name| { - match name.as_str() { - BUILT_IN_READ_ONLY_PROFILE => Some(PermissionProfile::read_only()), - BUILT_IN_WORKSPACE_PROFILE => Some(PermissionProfile::workspace_write()), - _ => None, - } - .map(|profile| profile.to_runtime_permissions()) - }); - let (mut file_system_sandbox_policy, base_network_sandbox_policy) = base_permissions - .unwrap_or_else(|| { - ( - FileSystemSandboxPolicy::restricted(Vec::new()), - NetworkSandboxPolicy::Restricted, - ) - }); + let ResolvedPermissionProfileToml { profile, .. } = + resolve_permission_profile(permissions, profile_name)?; + let mut file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(Vec::new()); + let base_network_sandbox_policy = NetworkSandboxPolicy::Restricted; if let Some(filesystem) = profile.filesystem.as_ref() { if filesystem.is_empty() && file_system_sandbox_policy.entries.is_empty() { push_warning( @@ -693,6 +784,7 @@ fn parse_special_path(path: &str) -> Option { ":minimal" => Some(FileSystemSpecialPath::Minimal), ":workspace_roots" => Some(FileSystemSpecialPath::project_roots(/*subpath*/ None)), ":tmpdir" => Some(FileSystemSpecialPath::Tmpdir), + ":slash_tmp" => Some(FileSystemSpecialPath::SlashTmp), _ if path.starts_with(':') => { Some(FileSystemSpecialPath::unknown(path, /*subpath*/ None)) } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index cbf8895ddb..5a08610cd6 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -374,16 +374,9 @@ impl Default for PermissionProfile { impl PermissionProfile { /// Managed read-only filesystem access with restricted network access. pub fn read_only() -> Self { + let file_system = FileSystemSandboxPolicy::read_only(); Self::Managed { - file_system: ManagedFileSystemPermissions::Restricted { - entries: vec![FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }], - glob_scan_max_depth: None, - }, + file_system: ManagedFileSystemPermissions::from_sandbox_policy(&file_system), network: NetworkSandboxPolicy::Restricted, } } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index f90859a92b..e5fbc5a0bb 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -358,22 +358,26 @@ pub fn project_roots_glob_pattern(subpath: &Path) -> String { format!("{PROJECT_ROOTS_GLOB_PATTERN_PREFIX}{}", subpath.display()) } +fn read_only_file_system_entries() -> Vec { + vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }] +} + impl Default for FileSystemSandboxPolicy { fn default() -> Self { - Self { - kind: FileSystemSandboxKind::Restricted, - glob_scan_max_depth: None, - entries: vec![FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }], - } + Self::read_only() } } impl FileSystemSandboxPolicy { + pub fn read_only() -> Self { + Self::restricted(read_only_file_system_entries()) + } + pub fn unrestricted() -> Self { Self { kind: FileSystemSandboxKind::Unrestricted,