diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 998f016c9671..80c00e75cd2d 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -2,6 +2,7 @@ use super::*; use codex_model_provider::SharedModelProvider; use codex_model_provider::create_model_provider; use codex_protocol::protocol::TurnEnvironmentSelection; +use codex_sandboxing::policy_transforms::merge_permission_profiles; pub(super) fn image_generation_tool_auth_allowed(auth_manager: Option<&AuthManager>) -> bool { matches!( @@ -83,6 +84,13 @@ pub(crate) struct TurnContext { pub(crate) turn_timing_state: Arc, } impl TurnContext { + pub(crate) fn permission_profile(&self) -> PermissionProfile { + PermissionProfile::from_runtime_permissions( + &self.file_system_sandbox_policy, + self.network_sandbox_policy, + ) + } + pub(crate) fn model_context_window(&self) -> Option { let effective_context_window_percent = self.model_info.effective_context_window_percent; self.model_info @@ -220,17 +228,19 @@ impl TurnContext { &self, additional_permissions: Option, ) -> FileSystemSandboxContext { + let base_permissions = self.permission_profile(); + let permissions = + merge_permission_profiles(Some(&base_permissions), additional_permissions.as_ref()) + .unwrap_or(base_permissions); FileSystemSandboxContext { - sandbox_policy: self.sandbox_policy.get().clone(), - sandbox_policy_cwd: Some(self.cwd.clone()), - file_system_sandbox_policy: self.non_legacy_file_system_sandbox_policy(), + permissions, + cwd: Some(self.cwd.clone()), windows_sandbox_level: self.windows_sandbox_level, windows_sandbox_private_desktop: self .config .permissions .windows_sandbox_private_desktop, use_legacy_landlock: self.features.use_legacy_landlock(), - additional_permissions, } } diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 1d9454be9c08..afc8751fa904 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -24,7 +24,6 @@ use codex_protocol::error::SandboxErr; use codex_protocol::exec_output::ExecToolCallOutput; use codex_protocol::exec_output::StreamOutput; use codex_protocol::models::PermissionProfile; -use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; @@ -34,6 +33,7 @@ use codex_protocol::protocol::FileChange; use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_utils_absolute_path::AbsolutePathBuf; use futures::future::BoxFuture; use std::path::PathBuf; @@ -77,22 +77,19 @@ impl ApplyPatchRuntime { return None; } - let legacy_file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( - attempt.policy, - attempt.sandbox_cwd, + let base_permissions = PermissionProfile::from_runtime_permissions( + attempt.file_system_policy, + attempt.network_policy, ); - let file_system_sandbox_policy = (attempt.file_system_policy - != &legacy_file_system_sandbox_policy) - .then(|| attempt.file_system_policy.clone()); - + let permissions = + merge_permission_profiles(Some(&base_permissions), req.additional_permissions.as_ref()) + .unwrap_or(base_permissions); Some(FileSystemSandboxContext { - sandbox_policy: attempt.policy.clone(), - sandbox_policy_cwd: Some(attempt.sandbox_cwd.clone()), - file_system_sandbox_policy, + permissions, + cwd: Some(attempt.sandbox_cwd.clone()), windows_sandbox_level: attempt.windows_sandbox_level, windows_sandbox_private_desktop: attempt.windows_sandbox_private_desktop, use_legacy_landlock: attempt.use_legacy_landlock, - additional_permissions: req.additional_permissions.clone(), }) } diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index 5e6359d3ec51..fea640b4482b 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -3,15 +3,13 @@ use crate::tools::sandboxing::SandboxAttempt; use codex_protocol::config_types::WindowsSandboxLevel; 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::GranularApprovalConfig; use codex_protocol::protocol::SandboxPolicy; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxType; +use codex_sandboxing::policy_transforms::merge_permission_profiles; use core_test_support::PathBufExt; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -135,12 +133,7 @@ fn file_system_sandbox_context_uses_active_attempt() { permissions_preapproved: false, }; let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let mut file_system_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, path.as_path()); - file_system_policy.entries.push(FileSystemSandboxEntry { - path: FileSystemPath::Path { path: path.clone() }, - access: FileSystemAccessMode::None, - }); + let file_system_policy = FileSystemSandboxPolicy::from(&sandbox_policy); let manager = SandboxManager::new(); let attempt = SandboxAttempt { sandbox: SandboxType::MacosSeatbelt, @@ -159,13 +152,17 @@ fn file_system_sandbox_context_uses_active_attempt() { let sandbox = ApplyPatchRuntime::file_system_sandbox_context_for_attempt(&req, &attempt) .expect("sandbox context"); - assert_eq!(sandbox.sandbox_policy, sandbox_policy); - assert_eq!(sandbox.sandbox_policy_cwd, Some(path.clone())); - assert_eq!( - sandbox.file_system_sandbox_policy, - Some(file_system_policy.clone()) + let base_permissions = PermissionProfile::from_runtime_permissions( + &file_system_policy, + NetworkSandboxPolicy::Restricted, ); - assert_eq!(sandbox.additional_permissions, Some(additional_permissions)); + let Some(expected_permissions) = + merge_permission_profiles(Some(&base_permissions), Some(&additional_permissions)) + else { + panic!("merged permissions should not be empty"); + }; + assert_eq!(sandbox.permissions, expected_permissions); + assert_eq!(sandbox.cwd, Some(path.clone())); assert_eq!( sandbox.windows_sandbox_level, WindowsSandboxLevel::RestrictedToken @@ -174,47 +171,6 @@ fn file_system_sandbox_context_uses_active_attempt() { assert_eq!(sandbox.use_legacy_landlock, true); } -#[test] -fn file_system_sandbox_context_omits_legacy_equivalent_policy() { - let path = std::env::temp_dir() - .join("apply-patch-runtime-legacy-equivalent.txt") - .abs(); - let req = ApplyPatchRequest { - action: ApplyPatchAction::new_add_for_test(&path, "hello".to_string()), - file_paths: vec![path.clone()], - changes: HashMap::new(), - exec_approval_requirement: ExecApprovalRequirement::Skip { - bypass_sandbox: false, - proposed_execpolicy_amendment: None, - }, - additional_permissions: None, - permissions_preapproved: false, - }; - let sandbox_policy = SandboxPolicy::new_read_only_policy(); - let file_system_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, path.as_path()); - let manager = SandboxManager::new(); - let attempt = SandboxAttempt { - sandbox: SandboxType::MacosSeatbelt, - policy: &sandbox_policy, - file_system_policy: &file_system_policy, - network_policy: NetworkSandboxPolicy::Restricted, - enforce_managed_network: false, - manager: &manager, - sandbox_cwd: &path, - codex_linux_sandbox_exe: None, - use_legacy_landlock: true, - windows_sandbox_level: WindowsSandboxLevel::RestrictedToken, - windows_sandbox_private_desktop: true, - }; - - let sandbox = ApplyPatchRuntime::file_system_sandbox_context_for_attempt(&req, &attempt) - .expect("sandbox context"); - - assert_eq!(sandbox.sandbox_policy_cwd, Some(path)); - assert_eq!(sandbox.file_system_sandbox_policy, None); -} - #[test] fn no_sandbox_attempt_has_no_file_system_context() { let path = std::env::temp_dir() diff --git a/codex-rs/exec-server/src/file_system.rs b/codex-rs/exec-server/src/file_system.rs index 64ebc891db46..77676548ffb8 100644 --- a/codex-rs/exec-server/src/file_system.rs +++ b/codex-rs/exec-server/src/file_system.rs @@ -1,7 +1,9 @@ use async_trait::async_trait; use codex_protocol::config_types::WindowsSandboxLevel; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemSandboxKind; use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use tokio::io; @@ -41,37 +43,65 @@ pub struct ReadDirectoryEntry { #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub struct FileSystemSandboxContext { - pub sandbox_policy: SandboxPolicy, + pub permissions: PermissionProfile, #[serde(default, skip_serializing_if = "Option::is_none")] - pub sandbox_policy_cwd: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub file_system_sandbox_policy: Option, + pub cwd: Option, pub windows_sandbox_level: WindowsSandboxLevel, #[serde(default)] pub windows_sandbox_private_desktop: bool, #[serde(default)] pub use_legacy_landlock: bool, - pub additional_permissions: Option, } impl FileSystemSandboxContext { pub fn new(sandbox_policy: SandboxPolicy) -> Self { + if let Ok(cwd) = AbsolutePathBuf::current_dir() { + Self::from_legacy_sandbox_policy(sandbox_policy, cwd) + } else { + let permissions = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::from(&sandbox_policy), + ); + Self::from_permission_profile(permissions) + } + } + + pub fn from_legacy_sandbox_policy(sandbox_policy: SandboxPolicy, cwd: AbsolutePathBuf) -> Self { + let permissions = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()), + NetworkSandboxPolicy::from(&sandbox_policy), + ); + Self::from_permission_profile_with_cwd(permissions, cwd) + } + + pub fn from_permission_profile(permissions: PermissionProfile) -> Self { + Self::from_permissions_and_cwd(permissions, /*cwd*/ None) + } + + pub fn from_permission_profile_with_cwd( + permissions: PermissionProfile, + cwd: AbsolutePathBuf, + ) -> Self { + Self::from_permissions_and_cwd(permissions, Some(cwd)) + } + + fn from_permissions_and_cwd( + permissions: PermissionProfile, + cwd: Option, + ) -> Self { Self { - sandbox_policy, - sandbox_policy_cwd: None, - file_system_sandbox_policy: None, + permissions, + cwd, windows_sandbox_level: WindowsSandboxLevel::Disabled, windows_sandbox_private_desktop: false, use_legacy_landlock: false, - additional_permissions: None, } } pub fn should_run_in_sandbox(&self) -> bool { - matches!( - self.sandbox_policy, - SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } - ) + let file_system_policy = self.permissions.file_system_sandbox_policy(); + matches!(file_system_policy.kind, FileSystemSandboxKind::Restricted) + && !file_system_policy.has_full_disk_write_access() } } diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index c29d6b76537c..5f347e6e1c4c 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -1,9 +1,11 @@ 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::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; @@ -12,7 +14,6 @@ use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxablePreference; -use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; use tokio::io::AsyncWriteExt; @@ -36,13 +37,6 @@ pub(crate) struct FileSystemSandboxRunner { helper_env: HashMap, } -struct HelperSandboxInputs { - sandbox_policy: SandboxPolicy, - file_system_policy: FileSystemSandboxPolicy, - network_policy: NetworkSandboxPolicy, - cwd: AbsolutePathBuf, -} - impl FileSystemSandboxRunner { pub(crate) fn new(runtime_paths: ExecServerRuntimePaths) -> Self { Self { @@ -56,12 +50,18 @@ impl FileSystemSandboxRunner { sandbox: &FileSystemSandboxContext, request: FsHelperRequest, ) -> Result { - let HelperSandboxInputs { - sandbox_policy, - file_system_policy, - network_policy, - cwd, - } = helper_sandbox_inputs(sandbox)?; + let cwd = sandbox_cwd(sandbox)?; + let mut file_system_policy = sandbox.permissions.file_system_sandbox_policy(); + let helper_read_root = if sandbox.use_legacy_landlock { + None + } else { + helper_read_root(&self.runtime_paths) + }; + add_helper_runtime_permissions(&mut file_system_policy, helper_read_root, cwd.as_path()); + normalize_file_system_policy_root_aliases(&mut file_system_policy); + let network_policy = NetworkSandboxPolicy::Restricted; + let sandbox_policy = + compatibility_sandbox_policy(&file_system_policy, network_policy, cwd.as_path()); let command = self.sandbox_exec_request( &sandbox_policy, &file_system_policy, @@ -95,10 +95,7 @@ impl FileSystemSandboxRunner { args: vec![CODEX_FS_HELPER_ARG1.to_string()], cwd: cwd.clone(), env: self.helper_env.clone(), - additional_permissions: self.helper_permissions( - sandbox_context.additional_permissions.as_ref(), - /*include_helper_read_root*/ !sandbox_context.use_legacy_landlock, - ), + additional_permissions: None, }; sandbox_manager .transform(SandboxTransformRequest { @@ -117,99 +114,125 @@ impl FileSystemSandboxRunner { }) .map_err(|err| invalid_request(format!("failed to prepare fs sandbox: {err}"))) } +} - fn helper_permissions( - &self, - additional_permissions: Option<&PermissionProfile>, - include_helper_read_root: bool, - ) -> Option { - let inherited_permissions = additional_permissions - .map(|permissions| PermissionProfile { - network: None, - file_system: permissions.file_system.clone(), - }) - .filter(|permissions| !permissions.is_empty()); - let helper_permissions = include_helper_read_root - .then(|| { - self.runtime_paths - .codex_self_exe - .parent() - .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) - }) - .flatten() - .map(|helper_read_root| PermissionProfile { - network: None, - file_system: Some(FileSystemPermissions::from_read_write_roots( - Some(vec![helper_read_root]), - /*write*/ None, - )), - }); +fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result { + if let Some(cwd) = &sandbox.cwd { + return Ok(cwd.clone()); + } - merge_permission_profiles(inherited_permissions.as_ref(), helper_permissions.as_ref()) + let file_system_policy = sandbox.permissions.file_system_sandbox_policy(); + if file_system_policy_has_cwd_dependent_entries(&file_system_policy) { + return Err(invalid_request( + "file system sandbox context with cwd-relative permissions requires cwd".to_string(), + )); } + + let cwd = current_sandbox_cwd().map_err(io_error)?; + AbsolutePathBuf::from_absolute_path(cwd.as_path()) + .map_err(|err| invalid_request(format!("current directory is not absolute: {err}"))) } -fn helper_sandbox_inputs( - sandbox: &FileSystemSandboxContext, -) -> Result { - let sandbox_policy = normalize_sandbox_policy_root_aliases( - sandbox_policy_with_helper_runtime_defaults(&sandbox.sandbox_policy), - ); - let cwd = match &sandbox.sandbox_policy_cwd { - Some(cwd) => cwd.clone(), - None if sandbox.file_system_sandbox_policy.is_some() => { - return Err(invalid_request( - "fileSystemSandboxPolicy requires sandboxPolicyCwd".to_string(), - )); - } - None => { - let cwd = current_sandbox_cwd().map_err(io_error)?; - AbsolutePathBuf::from_absolute_path(cwd.as_path()).map_err(|err| { - invalid_request(format!("current directory is not absolute: {err}")) - })? +fn file_system_policy_has_cwd_dependent_entries( + file_system_policy: &FileSystemSandboxPolicy, +) -> bool { + file_system_policy + .entries + .iter() + .any(|entry| match &entry.path { + FileSystemPath::GlobPattern { pattern } => !std::path::Path::new(pattern).is_absolute(), + FileSystemPath::Special { + value: + FileSystemSpecialPath::CurrentWorkingDirectory + | FileSystemSpecialPath::ProjectRoots { .. }, + } => true, + FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false, + }) +} + +fn helper_read_root(runtime_paths: &ExecServerRuntimePaths) -> Option { + runtime_paths + .codex_self_exe + .parent() + .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) +} + +fn add_helper_runtime_permissions( + file_system_policy: &mut FileSystemSandboxPolicy, + helper_read_root: Option, + cwd: &std::path::Path, +) { + if !file_system_policy.has_full_disk_read_access() { + let minimal_read_entry = FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::Minimal, + }, + access: FileSystemAccessMode::Read, + }; + if !file_system_policy.entries.contains(&minimal_read_entry) { + file_system_policy.entries.push(minimal_read_entry); } + } + + let Some(helper_read_root) = helper_read_root else { + return; }; - let file_system_policy = sandbox - .file_system_sandbox_policy - .clone() - .unwrap_or_else(|| { - FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()) - }); - Ok(HelperSandboxInputs { - sandbox_policy, - file_system_policy, - network_policy: NetworkSandboxPolicy::Restricted, - cwd, - }) + if file_system_policy.can_read_path_with_cwd(helper_read_root.as_path(), cwd) { + return; + } + + file_system_policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: helper_read_root, + }, + access: FileSystemAccessMode::Read, + }); } -fn normalize_sandbox_policy_root_aliases(sandbox_policy: SandboxPolicy) -> SandboxPolicy { - let mut sandbox_policy = sandbox_policy; - match &mut sandbox_policy { - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::Restricted { readable_roots, .. }, - .. - } => { - normalize_root_aliases(readable_roots); - } - SandboxPolicy::WorkspaceWrite { - writable_roots, - read_only_access, - .. - } => { - normalize_root_aliases(writable_roots); - if let ReadOnlyAccess::Restricted { readable_roots, .. } = read_only_access { - normalize_root_aliases(readable_roots); - } +fn compatibility_sandbox_policy( + file_system_policy: &FileSystemSandboxPolicy, + network_policy: NetworkSandboxPolicy, + cwd: &std::path::Path, +) -> SandboxPolicy { + file_system_policy + .to_legacy_sandbox_policy(network_policy, cwd) + .unwrap_or_else(|_| compatibility_workspace_write_policy(file_system_policy, cwd)) +} + +fn compatibility_workspace_write_policy( + file_system_policy: &FileSystemSandboxPolicy, + cwd: &std::path::Path, +) -> SandboxPolicy { + let read_only_access = if file_system_policy.has_full_disk_read_access() { + ReadOnlyAccess::FullAccess + } else { + ReadOnlyAccess::Restricted { + include_platform_defaults: file_system_policy.include_platform_defaults(), + readable_roots: file_system_policy.get_readable_roots_with_cwd(cwd), } - _ => {} + }; + let cwd_abs = AbsolutePathBuf::from_absolute_path(cwd).ok(); + let writable_roots = file_system_policy + .get_writable_roots_with_cwd(cwd) + .into_iter() + .map(|root| root.root) + .filter(|root| cwd_abs.as_ref() != Some(root)) + .collect(); + + SandboxPolicy::WorkspaceWrite { + writable_roots, + read_only_access, + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, } - sandbox_policy } -fn normalize_root_aliases(paths: &mut Vec) { - for path in paths { - *path = normalize_top_level_alias(path.clone()); +fn normalize_file_system_policy_root_aliases(file_system_policy: &mut FileSystemSandboxPolicy) { + for entry in &mut file_system_policy.entries { + if let FileSystemPath::Path { path } = &mut entry.path { + *path = normalize_top_level_alias(path.clone()); + } } } @@ -314,39 +337,6 @@ fn spawn_command( command.spawn().map_err(io_error) } -fn sandbox_policy_with_helper_runtime_defaults(sandbox_policy: &SandboxPolicy) -> SandboxPolicy { - let mut sandbox_policy = sandbox_policy.clone(); - match &mut sandbox_policy { - SandboxPolicy::ReadOnly { - access, - network_access, - } => { - enable_platform_defaults(access); - *network_access = false; - } - SandboxPolicy::WorkspaceWrite { - read_only_access, - network_access, - .. - } => { - enable_platform_defaults(read_only_access); - *network_access = false; - } - SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => {} - } - sandbox_policy -} - -fn enable_platform_defaults(access: &mut ReadOnlyAccess) { - if let ReadOnlyAccess::Restricted { - include_platform_defaults, - .. - } = access - { - *include_platform_defaults = true; - } -} - fn io_error(err: std::io::Error) -> JSONRPCErrorError { internal_error(err.to_string()) } @@ -362,10 +352,12 @@ mod tests { use std::collections::HashMap; use std::ffi::OsString; - 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::FileSystemSandboxPolicy; + use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; @@ -373,17 +365,19 @@ mod tests { use pretty_assertions::assert_eq; use crate::ExecServerRuntimePaths; - use crate::FileSystemSandboxContext; use super::FileSystemSandboxRunner; + use super::add_helper_runtime_permissions; use super::helper_env; use super::helper_env_from_vars; use super::helper_env_key_is_allowed; - use super::helper_sandbox_inputs; - use super::sandbox_policy_with_helper_runtime_defaults; + use super::helper_read_root; + use super::sandbox_cwd; #[test] - fn helper_sandbox_policy_enables_platform_defaults_for_read_only_access() { + fn helper_permissions_enable_minimal_reads_for_read_only_access() { + let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + .expect("absolute cwd"); let sandbox_policy = SandboxPolicy::ReadOnly { access: ReadOnlyAccess::Restricted { include_platform_defaults: false, @@ -391,136 +385,76 @@ mod tests { }, network_access: false, }; + let mut policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); - let updated = sandbox_policy_with_helper_runtime_defaults(&sandbox_policy); + add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path()); - assert_eq!( - updated, - SandboxPolicy::ReadOnly { - access: ReadOnlyAccess::Restricted { - include_platform_defaults: true, - readable_roots: Vec::new(), - }, - network_access: false, - } - ); + assert!(policy.include_platform_defaults()); } #[test] - fn helper_sandbox_policy_enables_platform_defaults_for_workspace_read_access() { + fn helper_permissions_enable_minimal_reads_for_workspace_read_access() { + let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + .expect("absolute cwd"); let sandbox_policy = SandboxPolicy::WorkspaceWrite { writable_roots: Vec::new(), read_only_access: ReadOnlyAccess::Restricted { include_platform_defaults: false, readable_roots: Vec::new(), }, - network_access: true, + network_access: false, exclude_tmpdir_env_var: true, exclude_slash_tmp: true, }; + let mut policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); - let updated = sandbox_policy_with_helper_runtime_defaults(&sandbox_policy); + add_helper_runtime_permissions(&mut policy, /*helper_read_root*/ None, cwd.as_path()); - assert_eq!( - updated, - SandboxPolicy::WorkspaceWrite { - writable_roots: Vec::new(), - read_only_access: ReadOnlyAccess::Restricted { - include_platform_defaults: true, - readable_roots: Vec::new(), - }, - network_access: false, - exclude_tmpdir_env_var: true, - exclude_slash_tmp: true, - } - ); + assert!(policy.include_platform_defaults()); } #[test] - fn helper_sandbox_inputs_use_context_cwd_and_file_system_policy() { - let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) - .expect("absolute temp dir"); - let sandbox_policy = SandboxPolicy::new_workspace_write_policy(); - let file_system_policy = - codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy( - &sandbox_policy, - cwd.as_path(), - ); - let mut sandbox_context = FileSystemSandboxContext::new(sandbox_policy.clone()); - sandbox_context.sandbox_policy_cwd = Some(cwd.clone()); - sandbox_context.file_system_sandbox_policy = Some(file_system_policy.clone()); - - let inputs = helper_sandbox_inputs(&sandbox_context).expect("helper sandbox inputs"); - - assert_eq!(inputs.cwd, cwd); - assert_eq!(inputs.sandbox_policy, sandbox_policy); - assert_eq!(inputs.file_system_policy, file_system_policy); - assert_eq!(inputs.network_policy, NetworkSandboxPolicy::Restricted); - } - - #[test] - fn helper_sandbox_inputs_rejects_file_system_policy_without_cwd() { + fn helper_permissions_preserve_existing_writes() { + let codex_self_exe = std::env::current_exe().expect("current exe"); + let runtime_paths = + ExecServerRuntimePaths::new(codex_self_exe, /*codex_linux_sandbox_exe*/ None) + .expect("runtime paths"); let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) - .expect("absolute temp dir"); - let sandbox_policy = SandboxPolicy::new_workspace_write_policy(); - let file_system_policy = - codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy( - &sandbox_policy, - cwd.as_path(), - ); - let mut sandbox_context = FileSystemSandboxContext::new(sandbox_policy); - sandbox_context.file_system_sandbox_policy = Some(file_system_policy); - - let err = match helper_sandbox_inputs(&sandbox_context) { - Ok(_) => panic!("expected invalid sandbox inputs"), - Err(err) => err, + .expect("absolute cwd"); + let writable = cwd.join("writable"); + let sandbox_policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: true, }; - - assert_eq!( - err.message, - "fileSystemSandboxPolicy requires sandboxPolicyCwd" - ); - } - - #[test] - fn helper_permissions_strip_network_grants() { - let codex_self_exe = std::env::current_exe().expect("current exe"); - let runtime_paths = ExecServerRuntimePaths::new( - codex_self_exe.clone(), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let runner = FileSystemSandboxRunner::new(runtime_paths); + let mut policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); + policy.entries.push(FileSystemSandboxEntry { + path: FileSystemPath::Path { + path: writable.clone(), + }, + access: FileSystemAccessMode::Write, + }); let readable = AbsolutePathBuf::from_absolute_path( - codex_self_exe.parent().expect("current exe parent"), + runtime_paths + .codex_self_exe + .parent() + .expect("current exe parent"), ) .expect("absolute readable path"); - let writable = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) - .expect("absolute writable path"); - - let permissions = runner - .helper_permissions( - Some(&PermissionProfile { - network: Some(NetworkPermissions { - enabled: Some(true), - }), - file_system: Some(FileSystemPermissions::from_read_write_roots( - Some(vec![]), - Some(vec![writable.clone()]), - )), - }), - /*include_helper_read_root*/ true, - ) - .expect("helper permissions"); - 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!(write, Some(vec![writable])); - assert_eq!(read, Some(vec![readable])); + + add_helper_runtime_permissions( + &mut policy, + helper_read_root(&runtime_paths), + cwd.as_path(), + ); + + assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path())); + assert!(policy.can_write_path_with_cwd(writable.as_path(), cwd.as_path())); } #[test] @@ -617,47 +551,70 @@ mod tests { } #[test] - fn helper_permissions_include_helper_read_root_without_additional_permissions() { - let codex_self_exe = std::env::current_exe().expect("current exe"); - let runtime_paths = ExecServerRuntimePaths::new( - codex_self_exe.clone(), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let runner = FileSystemSandboxRunner::new(runtime_paths); - let readable = AbsolutePathBuf::from_absolute_path( - codex_self_exe.parent().expect("current exe parent"), - ) - .expect("absolute readable path"); + fn sandbox_cwd_uses_context_cwd() { + let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + .expect("absolute cwd"); + let sandbox_context = crate::FileSystemSandboxContext::from_legacy_sandbox_policy( + SandboxPolicy::new_workspace_write_policy(), + cwd.clone(), + ); - let permissions = runner - .helper_permissions( - /*additional_permissions*/ None, /*include_helper_read_root*/ true, - ) - .expect("helper permissions"); + assert_eq!(sandbox_cwd(&sandbox_context).expect("sandbox cwd"), cwd); + } + + #[test] + fn sandbox_cwd_rejects_cwd_dependent_profile_without_context_cwd() { + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::CurrentWorkingDirectory, + }, + access: FileSystemAccessMode::Write, + }]); + let sandbox_context = + crate::FileSystemSandboxContext::from_permission_profile(PermissionProfile { + network: None, + file_system: Some((&policy).into()), + }); + + let err = sandbox_cwd(&sandbox_context).expect_err("missing cwd should be rejected"); - assert_eq!(permissions.network, None); assert_eq!( - permissions.file_system, - Some(FileSystemPermissions::from_read_write_roots( - Some(vec![readable]), - /*write*/ None, - )) + err.message, + "file system sandbox context with cwd-relative permissions requires cwd" ); } #[test] - fn legacy_landlock_helper_permissions_do_not_add_helper_read_root() { + fn helper_permissions_include_helper_read_root_without_additional_permissions() { let codex_self_exe = std::env::current_exe().expect("current exe"); let runtime_paths = ExecServerRuntimePaths::new(codex_self_exe, /*codex_linux_sandbox_exe*/ None) .expect("runtime paths"); - let runner = FileSystemSandboxRunner::new(runtime_paths); + let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + .expect("absolute cwd"); + let sandbox_policy = SandboxPolicy::ReadOnly { + access: ReadOnlyAccess::Restricted { + include_platform_defaults: false, + readable_roots: Vec::new(), + }, + network_access: false, + }; + let mut policy = + FileSystemSandboxPolicy::from_legacy_sandbox_policy(&sandbox_policy, cwd.as_path()); + let readable = AbsolutePathBuf::from_absolute_path( + runtime_paths + .codex_self_exe + .parent() + .expect("current exe parent"), + ) + .expect("absolute readable path"); - let permissions = runner.helper_permissions( - /*additional_permissions*/ None, /*include_helper_read_root*/ false, + add_helper_runtime_permissions( + &mut policy, + helper_read_root(&runtime_paths), + cwd.as_path(), ); - assert_eq!(permissions, None); + assert!(policy.can_read_path_with_cwd(readable.as_path(), cwd.as_path())); } } diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index f137be969580..fe0f9541f75e 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -25,6 +25,7 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; +use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -101,6 +102,31 @@ fn workspace_write_sandbox(writable_root: std::path::PathBuf) -> FileSystemSandb }) } +#[test] +fn sandbox_context_new_preserves_legacy_workspace_write_read_only_subpaths() -> Result<()> { + let tmp = TempDir::new()?; + let writable_dir = tmp.path().join("writable"); + let git_dir = writable_dir.join(".git"); + std::fs::create_dir_all(&git_dir)?; + + let sandbox = workspace_write_sandbox(writable_dir.clone()); + let cwd = sandbox.cwd.as_ref().expect("sandbox cwd"); + let policy = sandbox.permissions.file_system_sandbox_policy(); + let writable_roots = policy.get_writable_roots_with_cwd(cwd.as_path()); + let writable_dir = absolute_path(std::fs::canonicalize(writable_dir)?); + let git_dir = absolute_path(std::fs::canonicalize(git_dir)?); + let Some(writable_root) = writable_roots + .iter() + .find(|writable_root| writable_root.root == writable_dir) + else { + panic!("writable root should be preserved"); + }; + + assert!(writable_root.read_only_subpaths.contains(&git_dir)); + + Ok(()) +} + fn assert_sandbox_denied(error: &std::io::Error) { match error.kind() { std::io::ErrorKind::InvalidInput | std::io::ErrorKind::PermissionDenied => { @@ -567,13 +593,19 @@ async fn file_system_sandboxed_write_allows_additional_write_root(use_remote: bo std::fs::create_dir_all(&writable_dir)?; let mut sandbox = read_only_sandbox(readable_dir); - sandbox.additional_permissions = Some(PermissionProfile { + let additional_permissions = PermissionProfile { network: None, file_system: Some(FileSystemPermissions::from_read_write_roots( /*read*/ None, Some(vec![absolute_path(writable_dir)]), )), - }); + }; + let Some(permissions) = + merge_permission_profiles(Some(&sandbox.permissions), Some(&additional_permissions)) + else { + panic!("merged permissions should not be empty"); + }; + sandbox.permissions = permissions; file_system .write_file(