From f858b35bc1ab68bc8da4eb86ab07295d0c3c686e Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 26 Apr 2026 22:19:35 -0700 Subject: [PATCH] permissions: derive config defaults as profiles --- codex-rs/config/src/config_toml.rs | 83 +++++++----- codex-rs/core/src/config/mod.rs | 37 +++-- codex-rs/protocol/src/models.rs | 71 ++++------ codex-rs/protocol/src/permissions.rs | 194 ++++++++++++++++++++------- 4 files changed, 243 insertions(+), 142 deletions(-) diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index 2821de5a8b..71b55d9094 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -49,6 +49,7 @@ use codex_protocol::config_types::WebSearchToolConfig; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -641,15 +642,15 @@ pub struct GhostSnapshotToml { } impl ConfigToml { - /// Derive the effective sandbox policy from the configuration. - pub async fn derive_sandbox_policy( + /// Derive the effective permission profile from the configuration. + pub async fn derive_permission_profile( &self, sandbox_mode_override: Option, profile_sandbox_mode: Option, windows_sandbox_level: WindowsSandboxLevel, active_project: Option<&ProjectConfig>, permission_profile_constraint: Option<&crate::Constrained>, - ) -> SandboxPolicy { + ) -> PermissionProfile { let sandbox_mode_was_explicit = sandbox_mode_override.is_some() || profile_sandbox_mode.is_some() || self.sandbox_mode.is_some(); @@ -677,50 +678,72 @@ impl ConfigToml { }) }) .unwrap_or_default(); - let mut sandbox_policy = match resolved_sandbox_mode { - SandboxMode::ReadOnly => SandboxPolicy::new_read_only_policy(), + let workspace_write_unsupported = cfg!(target_os = "windows") + // If the experimental Windows sandbox is enabled, do not force a downgrade. + && windows_sandbox_level == WindowsSandboxLevel::Disabled + && matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite); + let mut permission_profile = match resolved_sandbox_mode { + SandboxMode::ReadOnly => PermissionProfile::read_only(), SandboxMode::WorkspaceWrite => match self.sandbox_workspace_write.as_ref() { Some(SandboxWorkspaceWrite { writable_roots, network_access, exclude_tmpdir_env_var, exclude_slash_tmp, - }) => SandboxPolicy::WorkspaceWrite { - writable_roots: writable_roots.clone(), - network_access: *network_access, - exclude_tmpdir_env_var: *exclude_tmpdir_env_var, - exclude_slash_tmp: *exclude_slash_tmp, - }, - None => SandboxPolicy::new_workspace_write_policy(), + }) => { + let network_policy = if *network_access { + NetworkSandboxPolicy::Enabled + } else { + NetworkSandboxPolicy::Restricted + }; + PermissionProfile::workspace_write_with( + writable_roots, + network_policy, + *exclude_tmpdir_env_var, + *exclude_slash_tmp, + ) + } + None => PermissionProfile::workspace_write(), }, - SandboxMode::DangerFullAccess => SandboxPolicy::DangerFullAccess, + SandboxMode::DangerFullAccess => PermissionProfile::Disabled, }; - let downgrade_workspace_write_if_unsupported = |policy: &mut SandboxPolicy| { - if cfg!(target_os = "windows") - // If the experimental Windows sandbox is enabled, do not force a downgrade. - && windows_sandbox_level == WindowsSandboxLevel::Disabled - && matches!(&*policy, SandboxPolicy::WorkspaceWrite { .. }) - { - *policy = SandboxPolicy::new_read_only_policy(); - } - }; - if matches!(resolved_sandbox_mode, SandboxMode::WorkspaceWrite) { - downgrade_workspace_write_if_unsupported(&mut sandbox_policy); + if workspace_write_unsupported { + permission_profile = PermissionProfile::read_only(); } if !sandbox_mode_was_explicit && let Some(constraint) = permission_profile_constraint - && let Err(err) = constraint.can_set(&PermissionProfile::from_legacy_sandbox_policy( - &sandbox_policy, - )) + && let Err(err) = constraint.can_set(&permission_profile) { tracing::warn!( error = %err, "default sandbox policy is disallowed by requirements; falling back to required default" ); - sandbox_policy = SandboxPolicy::new_read_only_policy(); - downgrade_workspace_write_if_unsupported(&mut sandbox_policy); + permission_profile = PermissionProfile::read_only(); } - sandbox_policy + permission_profile + } + + /// Derive the legacy sandbox projection from configuration. + /// + /// New callers should use [`Self::derive_permission_profile`] instead. + pub async fn derive_sandbox_policy( + &self, + sandbox_mode_override: Option, + profile_sandbox_mode: Option, + windows_sandbox_level: WindowsSandboxLevel, + active_project: Option<&ProjectConfig>, + permission_profile_constraint: Option<&crate::Constrained>, + ) -> SandboxPolicy { + self.derive_permission_profile( + sandbox_mode_override, + profile_sandbox_mode, + windows_sandbox_level, + active_project, + permission_profile_constraint, + ) + .await + .to_legacy_sandbox_policy(Path::new("/")) + .expect("legacy config-derived permission profile should be bridgeable") } /// Resolves the cwd to an existing project, or returns None if ConfigToml diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 9635034dcc..55d4a06d35 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1965,8 +1965,8 @@ impl Config { ) } else { let configured_network_proxy_config = NetworkProxyConfig::default(); - let mut sandbox_policy = cfg - .derive_sandbox_policy( + let mut permission_profile = cfg + .derive_permission_profile( sandbox_mode, config_profile.sandbox_mode, windows_sandbox_level, @@ -1974,24 +1974,23 @@ impl Config { Some(&constrained_permission_profile), ) .await; - if let SandboxPolicy::WorkspaceWrite { writable_roots, .. } = &mut sandbox_policy { - for path in &additional_writable_roots { - if !writable_roots.iter().any(|existing| existing == path) { - writable_roots.push(path.clone()); - } - } + let (mut file_system_sandbox_policy, network_sandbox_policy) = + permission_profile.to_runtime_permissions(); + if matches!(permission_profile.enforcement(), SandboxEnforcement::Managed) + && file_system_sandbox_policy.can_write_path_with_cwd( + resolved_cwd.as_path(), + resolved_cwd.as_path(), + ) + && !file_system_sandbox_policy.has_full_disk_write_access() + { + file_system_sandbox_policy = file_system_sandbox_policy + .with_additional_legacy_workspace_writable_roots(&additional_writable_roots); + permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( + permission_profile.enforcement(), + &file_system_sandbox_policy, + network_sandbox_policy, + ); } - let file_system_sandbox_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( - &sandbox_policy, - resolved_cwd.as_path(), - ); - let network_sandbox_policy = NetworkSandboxPolicy::from(&sandbox_policy); - let permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( - SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ); ( configured_network_proxy_config, permission_profile, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 7da986efda..0e71ccab77 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -404,55 +404,30 @@ impl PermissionProfile { /// Managed workspace-write filesystem access with restricted network access. pub fn workspace_write() -> Self { + Self::workspace_write_with( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ) + } + + /// Managed workspace-write filesystem access with the legacy + /// `sandbox_workspace_write` knobs applied directly to the profile. + pub fn workspace_write_with( + writable_roots: &[AbsolutePathBuf], + network: NetworkSandboxPolicy, + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, + ) -> Self { + let file_system = FileSystemSandboxPolicy::legacy_workspace_write( + writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, + ); Self::Managed { - file_system: ManagedFileSystemPermissions::Restricted { - entries: vec![ - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::CurrentWorkingDirectory, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::SlashTmp, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Tmpdir, - }, - access: FileSystemAccessMode::Write, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".git".into())), - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".agents".into())), - }, - access: FileSystemAccessMode::Read, - }, - FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::project_roots(Some(".codex".into())), - }, - access: FileSystemAccessMode::Read, - }, - ], - glob_scan_max_depth: None, - }, - network: NetworkSandboxPolicy::Restricted, + file_system: ManagedFileSystemPermissions::from_sandbox_policy(&file_system), + network, } } diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 63c2e3c4f7..12fca0071b 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -418,7 +418,6 @@ impl FileSystemSandboxPolicy { /// can preserve the active cwd binding until the policy is actually /// resolved for a turn or command. pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy) -> Self { - let mut file_system_policy = Self::from(sandbox_policy); let SandboxPolicy::WorkspaceWrite { writable_roots, exclude_tmpdir_env_var, @@ -426,15 +425,31 @@ impl FileSystemSandboxPolicy { .. } = sandbox_policy else { - return file_system_policy; + return Self::from(sandbox_policy); }; + Self::legacy_workspace_write(writable_roots, *exclude_tmpdir_env_var, *exclude_slash_tmp) + } + + /// Filesystem policy matching legacy `WorkspaceWrite` semantics without + /// requiring callers to construct a legacy [`SandboxPolicy`] first. + pub fn legacy_workspace_write( + writable_roots: &[AbsolutePathBuf], + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, + ) -> Self { + let mut file_system_policy = legacy_workspace_write_base_policy( + writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, + ); + prune_read_entries_under_writable_roots( &mut file_system_policy.entries, &legacy_non_cwd_writable_roots( writable_roots, - *exclude_tmpdir_env_var, - *exclude_slash_tmp, + exclude_tmpdir_env_var, + exclude_slash_tmp, ), ); @@ -613,6 +628,44 @@ impl FileSystemSandboxPolicy { self } + /// Add roots using legacy `WorkspaceWrite` behavior. + /// + /// Unlike [`Self::with_additional_writable_roots`], this mirrors legacy + /// writable-roots semantics by adding exact roots even when they are + /// already writable through `:cwd`, and by adding the default read-only + /// protected subpaths for each new root. + pub fn with_additional_legacy_workspace_writable_roots( + mut self, + additional_writable_roots: &[AbsolutePathBuf], + ) -> Self { + if !matches!(self.kind, FileSystemSandboxKind::Restricted) { + return self; + } + + for path in additional_writable_roots { + if !self.entries.iter().any(|entry| { + entry.access.can_write() + && matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == path) + }) { + self.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { path: path.clone() }, + access: FileSystemAccessMode::Write, + }); + } + + for protected_path in default_read_only_subpaths_for_writable_root( + path, /*protect_missing_dot_codex*/ false, + ) { + append_default_read_only_path_if_no_explicit_rule( + &mut self.entries, + protected_path, + ); + } + } + + self + } + pub fn needs_direct_runtime_enforcement( &self, network_policy: NetworkSandboxPolicy, @@ -991,51 +1044,61 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy { exclude_tmpdir_env_var, exclude_slash_tmp, .. - } => { - let mut entries = vec![FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Root, - }, - access: FileSystemAccessMode::Read, - }]; - - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::CurrentWorkingDirectory, - }, - access: FileSystemAccessMode::Write, - }); - if !exclude_slash_tmp { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::SlashTmp, - }, - access: FileSystemAccessMode::Write, - }); - } - if !exclude_tmpdir_env_var { - entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Special { - value: FileSystemSpecialPath::Tmpdir, - }, - access: FileSystemAccessMode::Write, - }); - } - entries.extend( - writable_roots - .iter() - .cloned() - .map(|path| FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, - access: FileSystemAccessMode::Write, - }), - ); - FileSystemSandboxPolicy::restricted(entries) - } + } => legacy_workspace_write_base_policy( + writable_roots, + *exclude_tmpdir_env_var, + *exclude_slash_tmp, + ), } } } +fn legacy_workspace_write_base_policy( + writable_roots: &[AbsolutePathBuf], + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, +) -> FileSystemSandboxPolicy { + let mut entries = vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + access: FileSystemAccessMode::Read, + }]; + + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }); + if !exclude_slash_tmp { + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::SlashTmp, + }, + access: FileSystemAccessMode::Write, + }); + } + if !exclude_tmpdir_env_var { + entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Tmpdir, + }, + access: FileSystemAccessMode::Write, + }); + } + entries.extend( + writable_roots + .iter() + .cloned() + .map(|path| FileSystemSandboxEntry { + path: FileSystemPath::Path { path }, + access: FileSystemAccessMode::Write, + }), + ); + FileSystemSandboxPolicy::restricted(entries) +} + fn resolve_file_system_path( path: &FileSystemPath, cwd: Option<&AbsolutePathBuf>, @@ -2380,6 +2443,47 @@ mod tests { ); } + #[test] + fn with_additional_legacy_workspace_writable_roots_protects_metadata() { + let temp_dir = TempDir::new().expect("tempdir"); + let extra = AbsolutePathBuf::from_absolute_path(temp_dir.path().join("extra")) + .expect("resolve extra root"); + std::fs::create_dir_all(extra.join(".git")).expect("create .git dir"); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + + let actual = + policy.with_additional_legacy_workspace_writable_roots(std::slice::from_ref(&extra)); + + assert_eq!( + actual, + FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: extra.clone() + }, + access: FileSystemAccessMode::Write, + }, + FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: extra.join(".git") + }, + access: FileSystemAccessMode::Read, + }, + ]) + ); + } + #[test] fn file_system_access_mode_orders_by_conflict_precedence() { assert!(FileSystemAccessMode::Write > FileSystemAccessMode::Read);