diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index fd2623f07b..4176abd47e 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -110,6 +110,7 @@ use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationResponse; use codex_rollout::RolloutConfig; use codex_rollout::state_db; +use codex_sandboxing::policy_transforms::merge_permission_profiles; use codex_shell_command::parse_command::parse_command; use codex_terminal_detection::user_agent; use codex_thread_store::LocalThreadStore; @@ -787,6 +788,323 @@ pub(crate) fn session_loop_termination_from_handle( .shared() } +/// Context for an initialized model agent +/// +/// A session has at most 1 running task at a time, and can be interrupted by user input. +pub(crate) struct Session { + pub(crate) conversation_id: ThreadId, + tx_event: Sender, + agent_status: watch::Sender, + out_of_band_elicitation_paused: watch::Sender, + state: Mutex, + /// Serializes rebuild/apply cycles for the running proxy; each cycle + /// rebuilds from the current SessionState while holding this lock. + managed_network_proxy_refresh_lock: Mutex<()>, + /// The set of enabled features should be invariant for the lifetime of the + /// session. + features: ManagedFeatures, + pending_mcp_server_refresh_config: Mutex>, + pub(crate) conversation: Arc, + pub(crate) active_turn: Mutex>, + mailbox: Mailbox, + mailbox_rx: Mutex, + idle_pending_input: Mutex>, // TODO (jif) merge with mailbox! + pub(crate) guardian_review_session: GuardianReviewSessionManager, + pub(crate) services: SessionServices, + js_repl: Arc, + next_internal_sub_id: AtomicU64, +} + +#[derive(Clone, Debug)] +pub(crate) struct TurnSkillsContext { + pub(crate) outcome: Arc, + pub(crate) implicit_invocation_seen_skills: Arc>>, +} + +impl TurnSkillsContext { + pub(crate) fn new(outcome: Arc) -> Self { + Self { + outcome, + implicit_invocation_seen_skills: Arc::new(Mutex::new(HashSet::new())), + } + } +} + +/// The context needed for a single turn of the thread. +#[derive(Debug)] +pub(crate) struct TurnContext { + pub(crate) sub_id: String, + pub(crate) trace_id: Option, + pub(crate) realtime_active: bool, + pub(crate) config: Arc, + pub(crate) auth_manager: Option>, + pub(crate) model_info: ModelInfo, + pub(crate) session_telemetry: SessionTelemetry, + pub(crate) provider: ModelProviderInfo, + pub(crate) reasoning_effort: Option, + pub(crate) reasoning_summary: ReasoningSummaryConfig, + pub(crate) session_source: SessionSource, + pub(crate) environment: Option>, + /// The session's absolute working directory. All relative paths provided + /// by the model as well as sandbox policies are resolved against this path + /// instead of `std::env::current_dir()`. + pub(crate) cwd: AbsolutePathBuf, + pub(crate) current_date: Option, + pub(crate) timezone: Option, + pub(crate) app_server_client_name: Option, + pub(crate) developer_instructions: Option, + pub(crate) compact_prompt: Option, + pub(crate) user_instructions: Option, + pub(crate) collaboration_mode: CollaborationMode, + pub(crate) personality: Option, + pub(crate) approval_policy: Constrained, + pub(crate) sandbox_policy: Constrained, + pub(crate) file_system_sandbox_policy: FileSystemSandboxPolicy, + pub(crate) network_sandbox_policy: NetworkSandboxPolicy, + pub(crate) network: Option, + pub(crate) windows_sandbox_level: WindowsSandboxLevel, + pub(crate) shell_environment_policy: ShellEnvironmentPolicy, + pub(crate) tools_config: ToolsConfig, + pub(crate) features: ManagedFeatures, + pub(crate) ghost_snapshot: GhostSnapshotConfig, + pub(crate) final_output_json_schema: Option, + pub(crate) codex_self_exe: Option, + pub(crate) codex_linux_sandbox_exe: Option, + pub(crate) tool_call_gate: Arc, + pub(crate) truncation_policy: TruncationPolicy, + pub(crate) js_repl: Arc, + pub(crate) dynamic_tools: Vec, + pub(crate) turn_metadata_state: Arc, + pub(crate) turn_skills: TurnSkillsContext, + pub(crate) turn_timing_state: Arc, +} +impl TurnContext { + pub(crate) fn model_context_window(&self) -> Option { + let effective_context_window_percent = self.model_info.effective_context_window_percent; + self.model_info.context_window.map(|context_window| { + context_window.saturating_mul(effective_context_window_percent) / 100 + }) + } + + pub(crate) fn apps_enabled(&self) -> bool { + let is_chatgpt_auth = self + .auth_manager + .as_deref() + .and_then(AuthManager::auth_cached) + .as_ref() + .is_some_and(CodexAuth::is_chatgpt_auth); + self.features.apps_enabled_for_auth(is_chatgpt_auth) + } + + pub(crate) async fn with_model(&self, model: String, models_manager: &ModelsManager) -> Self { + let mut config = (*self.config).clone(); + config.model = Some(model.clone()); + let model_info = models_manager + .get_model_info(model.as_str(), &config.to_models_manager_config()) + .await; + let truncation_policy = model_info.truncation_policy.into(); + let supported_reasoning_levels = model_info + .supported_reasoning_levels + .iter() + .map(|preset| preset.effort) + .collect::>(); + let reasoning_effort = if let Some(current_reasoning_effort) = self.reasoning_effort { + if supported_reasoning_levels.contains(¤t_reasoning_effort) { + Some(current_reasoning_effort) + } else { + supported_reasoning_levels + .get(supported_reasoning_levels.len().saturating_sub(1) / 2) + .copied() + .or(model_info.default_reasoning_level) + } + } else { + supported_reasoning_levels + .get(supported_reasoning_levels.len().saturating_sub(1) / 2) + .copied() + .or(model_info.default_reasoning_level) + }; + config.model_reasoning_effort = reasoning_effort; + + let collaboration_mode = self.collaboration_mode.with_updates( + Some(model.clone()), + Some(reasoning_effort), + /*developer_instructions*/ None, + ); + let features = self.features.clone(); + let tools_config = ToolsConfig::new(&ToolsConfigParams { + model_info: &model_info, + available_models: &models_manager + .list_models(RefreshStrategy::OnlineIfUncached) + .await, + features: &features, + image_generation_tool_auth_allowed: image_generation_tool_auth_allowed( + self.auth_manager.as_deref(), + ), + web_search_mode: self.tools_config.web_search_mode, + session_source: self.session_source.clone(), + sandbox_policy: self.sandbox_policy.get(), + windows_sandbox_level: self.windows_sandbox_level, + }) + .with_unified_exec_shell_mode(self.tools_config.unified_exec_shell_mode.clone()) + .with_web_search_config(self.tools_config.web_search_config.clone()) + .with_allow_login_shell(self.tools_config.allow_login_shell) + .with_has_environment(self.tools_config.has_environment) + .with_spawn_agent_usage_hint(config.multi_agent_v2.usage_hint_enabled) + .with_spawn_agent_usage_hint_text(config.multi_agent_v2.usage_hint_text.clone()) + .with_hide_spawn_agent_metadata(config.multi_agent_v2.hide_spawn_agent_metadata) + .with_agent_type_description(crate::agent::role::spawn_tool_spec::build( + &config.agent_roles, + )); + + Self { + sub_id: self.sub_id.clone(), + trace_id: self.trace_id.clone(), + realtime_active: self.realtime_active, + config: Arc::new(config), + auth_manager: self.auth_manager.clone(), + model_info: model_info.clone(), + session_telemetry: self + .session_telemetry + .clone() + .with_model(model.as_str(), model_info.slug.as_str()), + provider: self.provider.clone(), + reasoning_effort, + reasoning_summary: self.reasoning_summary, + session_source: self.session_source.clone(), + environment: self.environment.clone(), + cwd: self.cwd.clone(), + current_date: self.current_date.clone(), + timezone: self.timezone.clone(), + app_server_client_name: self.app_server_client_name.clone(), + developer_instructions: self.developer_instructions.clone(), + compact_prompt: self.compact_prompt.clone(), + user_instructions: self.user_instructions.clone(), + collaboration_mode, + personality: self.personality, + approval_policy: self.approval_policy.clone(), + sandbox_policy: self.sandbox_policy.clone(), + file_system_sandbox_policy: self.file_system_sandbox_policy.clone(), + network_sandbox_policy: self.network_sandbox_policy, + network: self.network.clone(), + windows_sandbox_level: self.windows_sandbox_level, + shell_environment_policy: self.shell_environment_policy.clone(), + tools_config, + features, + ghost_snapshot: self.ghost_snapshot.clone(), + final_output_json_schema: self.final_output_json_schema.clone(), + codex_self_exe: self.codex_self_exe.clone(), + codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(), + tool_call_gate: Arc::new(ReadinessFlag::new()), + truncation_policy, + js_repl: Arc::clone(&self.js_repl), + dynamic_tools: self.dynamic_tools.clone(), + turn_metadata_state: self.turn_metadata_state.clone(), + turn_skills: self.turn_skills.clone(), + turn_timing_state: Arc::clone(&self.turn_timing_state), + } + } + + pub(crate) fn resolve_path(&self, path: Option) -> AbsolutePathBuf { + path.as_ref() + .map_or_else(|| self.cwd.clone(), |path| self.cwd.join(path)) + } + + pub(crate) fn file_system_sandbox_context( + &self, + additional_permissions: Option, + ) -> FileSystemSandboxContext { + let base_permissions = PermissionProfile::from_runtime_permissions( + &self.file_system_sandbox_policy, + self.network_sandbox_policy, + ); + let permissions = + merge_permission_profiles(Some(&base_permissions), additional_permissions.as_ref()) + .unwrap_or(base_permissions); + FileSystemSandboxContext { + permissions, + 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(), + } + } + + pub(crate) fn compact_prompt(&self) -> &str { + self.compact_prompt + .as_deref() + .unwrap_or(compact::SUMMARIZATION_PROMPT) + } + + pub(crate) fn to_turn_context_item(&self) -> TurnContextItem { + let legacy_file_system_sandbox_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( + self.sandbox_policy.get(), + &self.cwd, + ); + // Omit the derived split filesystem policy when it is equivalent to + // the legacy sandbox policy. This keeps turn-context payloads stable + // while both fields exist; once callers consume only the split policy, + // this comparison and the legacy projection should go away. + let file_system_sandbox_policy = (self.file_system_sandbox_policy + != legacy_file_system_sandbox_policy) + .then(|| self.file_system_sandbox_policy.clone()); + + TurnContextItem { + turn_id: Some(self.sub_id.clone()), + trace_id: self.trace_id.clone(), + cwd: self.cwd.to_path_buf(), + current_date: self.current_date.clone(), + timezone: self.timezone.clone(), + approval_policy: self.approval_policy.value(), + sandbox_policy: self.sandbox_policy.get().clone(), + network: self.turn_context_network_item(), + file_system_sandbox_policy, + model: self.model_info.slug.clone(), + personality: self.personality, + collaboration_mode: Some(self.collaboration_mode.clone()), + realtime_active: Some(self.realtime_active), + effort: self.reasoning_effort, + summary: self.reasoning_summary, + user_instructions: self.user_instructions.clone(), + developer_instructions: self.developer_instructions.clone(), + final_output_json_schema: self.final_output_json_schema.clone(), + truncation_policy: Some(self.truncation_policy), + } + } + + fn turn_context_network_item(&self) -> Option { + let network = self + .config + .config_layer_stack + .requirements() + .network + .as_ref()?; + Some(TurnContextNetworkItem { + allowed_domains: network + .domains + .as_ref() + .and_then(codex_config::NetworkDomainPermissionsToml::allowed_domains) + .unwrap_or_default(), + denied_domains: network + .domains + .as_ref() + .and_then(codex_config::NetworkDomainPermissionsToml::denied_domains) + .unwrap_or_default(), + }) + } +} + +fn local_time_context() -> (String, String) { + match iana_time_zone::get_timezone() { + Ok(timezone) => (Local::now().format("%Y-%m-%d").to_string(), timezone), + Err(_) => ( + Utc::now().format("%Y-%m-%d").to_string(), + "Etc/UTC".to_string(), + ), + } +} + async fn thread_title_from_state_db( state_db: Option<&state_db::StateDbHandle>, codex_home: &AbsolutePathBuf, diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 1d5c8a1e36..756562264b 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -31,6 +31,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; @@ -74,12 +75,18 @@ impl ApplyPatchRuntime { return None; } + let base_permissions = PermissionProfile::from_runtime_permissions( + attempt.file_system_policy, + attempt.network_policy, + ); + let permissions = + merge_permission_profiles(Some(&base_permissions), req.additional_permissions.as_ref()) + .unwrap_or(base_permissions); Some(FileSystemSandboxContext { - sandbox_policy: attempt.policy.clone(), + permissions, 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 f838a43a7a..58adca2038 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -9,6 +9,7 @@ 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; @@ -118,8 +119,16 @@ 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.additional_permissions, Some(additional_permissions)); + let base_permissions = PermissionProfile::from_runtime_permissions( + &file_system_policy, + NetworkSandboxPolicy::Restricted, + ); + 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.windows_sandbox_level, WindowsSandboxLevel::RestrictedToken diff --git a/codex-rs/exec-server/src/file_system.rs b/codex-rs/exec-server/src/file_system.rs index b09347686a..6367ae95e8 100644 --- a/codex-rs/exec-server/src/file_system.rs +++ b/codex-rs/exec-server/src/file_system.rs @@ -1,6 +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; @@ -40,31 +43,36 @@ 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, 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 { + let permissions = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::from(&sandbox_policy), + NetworkSandboxPolicy::from(&sandbox_policy), + ); + Self::from_permission_profile(permissions) + } + + pub fn from_permission_profile(permissions: PermissionProfile) -> Self { Self { - sandbox_policy, + permissions, 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 87ca4be18a..054ba5353a 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -1,12 +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; @@ -45,19 +44,21 @@ impl FileSystemSandboxRunner { sandbox: &FileSystemSandboxContext, request: FsHelperRequest, ) -> Result { - let helper_sandbox_policy = normalize_sandbox_policy_root_aliases( - sandbox_policy_with_helper_runtime_defaults(&sandbox.sandbox_policy), - ); let cwd = current_sandbox_cwd().map_err(io_error)?; let cwd = AbsolutePathBuf::from_absolute_path(cwd.as_path()) .map_err(|err| invalid_request(format!("current directory is not absolute: {err}")))?; - let file_system_policy = FileSystemSandboxPolicy::from_legacy_sandbox_policy( - &helper_sandbox_policy, + let mut file_system_policy = sandbox.permissions.file_system_sandbox_policy(); + add_helper_runtime_permissions( + &mut file_system_policy, + helper_read_root(&self.runtime_paths), 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( - &helper_sandbox_policy, + &sandbox_policy, &file_system_policy, network_policy, &cwd, @@ -89,9 +90,7 @@ impl FileSystemSandboxRunner { args: vec![CODEX_FS_HELPER_ARG1.to_string()], cwd: cwd.clone(), env: HashMap::new(), - additional_permissions: Some( - self.helper_permissions(sandbox_context.additional_permissions.as_ref()), - ), + additional_permissions: None, }; sandbox_manager .transform(SandboxTransformRequest { @@ -110,74 +109,91 @@ impl FileSystemSandboxRunner { }) .map_err(|err| invalid_request(format!("failed to prepare fs sandbox: {err}"))) } +} - fn helper_permissions( - &self, - additional_permissions: Option<&PermissionProfile>, - ) -> PermissionProfile { - let helper_read_root = self - .runtime_paths - .codex_self_exe - .parent() - .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()); - let file_system = - match additional_permissions.and_then(|permissions| permissions.file_system.clone()) { - Some(mut file_system) => { - if let Some(helper_read_root) = &helper_read_root { - 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::from_read_write_roots( - Some(vec![helper_read_root]), - /*write*/ None, - ) - }), - }; +fn helper_read_root(runtime_paths: &ExecServerRuntimePaths) -> Option { + runtime_paths + .codex_self_exe + .parent() + .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) +} - PermissionProfile { - network: None, - file_system, +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; + }; + 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 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, + } } -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); +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()); } - 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); - } - } - _ => {} - } - sandbox_policy -} - -fn normalize_root_aliases(paths: &mut Vec) { - for path in paths { - *path = normalize_top_level_alias(path.clone()); } } @@ -262,28 +278,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, .. } => enable_platform_defaults(access), - SandboxPolicy::WorkspaceWrite { - read_only_access, .. - } => enable_platform_defaults(read_only_access), - 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()) } @@ -296,21 +290,23 @@ fn json_error(err: serde_json::Error) -> JSONRPCErrorError { #[cfg(test)] mod tests { - 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::protocol::ReadOnlyAccess; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; - use pretty_assertions::assert_eq; use crate::ExecServerRuntimePaths; - use super::FileSystemSandboxRunner; - use super::sandbox_policy_with_helper_runtime_defaults; + use super::add_helper_runtime_permissions; + use super::helper_read_root; #[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, @@ -318,23 +314,18 @@ 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 { @@ -345,83 +336,87 @@ mod tests { 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_permissions_strip_network_grants() { + 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.clone(), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let runner = FileSystemSandboxRunner::new(runtime_paths); + 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 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, + }; + 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()]), - )), - })); - 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"); + add_helper_runtime_permissions( + &mut policy, + helper_read_root(&runtime_paths), + cwd.as_path(), + ); - assert_eq!(permissions.network, None); - assert_eq!(write, Some(vec![writable])); - assert_eq!(read, Some(vec![readable])); + 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] 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 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 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( - codex_self_exe.parent().expect("current exe parent"), + runtime_paths + .codex_self_exe + .parent() + .expect("current exe parent"), ) .expect("absolute readable path"); - let permissions = runner.helper_permissions(/*additional_permissions*/ None); - - assert_eq!(permissions.network, None); - assert_eq!( - permissions.file_system, - Some(FileSystemPermissions::from_read_write_roots( - Some(vec![readable]), - None, - )) + 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())); } } diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 49c5cf4f05..d2ea5faa1f 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -23,6 +23,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; @@ -473,13 +474,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( 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(