diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 4fe5360277..c3b8f01786 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -2876,8 +2876,11 @@ fn mcp_server_status_serializes_absent_server_info_metadata_as_null() { #[test] fn sandbox_policy_round_trips_workspace_write_access() { + let directory = tempfile::tempdir().expect("tempdir"); + let root = AbsolutePathBuf::from_absolute_path(directory.path()).expect("absolute root"); + std::fs::create_dir(root.join(".git")).expect("git metadata"); let v2_policy = SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], + writable_roots: vec![root.clone(), root.clone()], network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, @@ -2887,13 +2890,39 @@ fn sandbox_policy_round_trips_workspace_write_access() { assert_eq!( core_policy, codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { - writable_roots: vec![], + writable_roots: vec![root.clone(), root], network_access: true, exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } ); + let profile = codex_protocol::models::PermissionProfile::from_legacy_sandbox_policy_template( + &v2_policy.to_core(), + ); + assert!( + !profile + .file_system_sandbox_policy() + .entries + .iter() + .any(|entry| { + entry.access == codex_protocol::permissions::FileSystemAccessMode::Read + && matches!( + entry.path, + codex_protocol::permissions::FileSystemPath::Path { .. } + ) + }) + ); + + assert_eq!( + SandboxPolicy::from( + profile + .to_legacy_sandbox_policy_template() + .expect("portable legacy template") + ), + v2_policy, + ); + let back_to_v2 = SandboxPolicy::from(core_policy); assert_eq!(back_to_v2, v2_policy); } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index dfe2008a5f..be6288a1bc 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -21,6 +21,7 @@ use crate::permissions::FileSystemSandboxEntry; use crate::permissions::FileSystemSandboxKind; use crate::permissions::FileSystemSandboxPolicy; use crate::permissions::FileSystemSpecialPath; +use crate::permissions::LegacySandboxPolicyProjection; use crate::permissions::NetworkSandboxPolicy; use crate::permissions::RawFileSystemSandboxEntry; use crate::protocol::SandboxPolicy; @@ -662,6 +663,28 @@ impl PermissionProfile { ) } + /// Decodes a portable legacy template without probing workspace metadata. + pub fn from_legacy_sandbox_policy_template(sandbox_policy: &SandboxPolicy) -> Self { + if let SandboxPolicy::WorkspaceWrite { + writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, + .. + } = sandbox_policy + { + Self::from_runtime_permissions( + &FileSystemSandboxPolicy::workspace_write_template( + writable_roots, + *exclude_tmpdir_env_var, + *exclude_slash_tmp, + ), + NetworkSandboxPolicy::from(sandbox_policy), + ) + } else { + Self::from_legacy_sandbox_policy(sandbox_policy) + } + } + pub fn from_legacy_sandbox_policy_for_cwd(sandbox_policy: &SandboxPolicy, cwd: &Path) -> Self { Self::from_runtime_permissions_with_enforcement( SandboxEnforcement::from_legacy_sandbox_policy(sandbox_policy), @@ -736,6 +759,23 @@ impl PermissionProfile { } } + /// Encodes an unresolved template without collapsing fixed roots into cwd access. + pub fn to_legacy_sandbox_policy_template(&self) -> io::Result { + let policy = self + .file_system_sandbox_policy() + .to_legacy_sandbox_policy_with_projection( + self.network_sandbox_policy(), + LegacySandboxPolicyProjection::Template, + )?; + if Self::from_legacy_sandbox_policy_template(&policy) != *self { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "permission profile cannot be represented by a legacy sandbox template", + )); + } + Ok(policy) + } + pub fn to_runtime_permissions(&self) -> (FileSystemSandboxPolicy, NetworkSandboxPolicy) { ( self.file_system_sandbox_policy(), diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index 6cc2839485..05999db7f7 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -40,6 +40,13 @@ pub use local_aliases::LocalFileSystemPolicyMatcher; pub use windows_glob::WindowsDenyReadGlobScan; pub use windows_glob::windows_deny_read_glob_scan; +/// Selects native execution projection or an unresolved, lossless transport template. +#[derive(Clone, Copy)] +pub(crate) enum LegacySandboxPolicyProjection<'a> { + Cwd(&'a Path), + Template, +} + const PROTECTED_METADATA_GIT_PATH_NAME: &str = ".git"; const PROTECTED_METADATA_AGENTS_PATH_NAME: &str = ".agents"; const PROTECTED_METADATA_CODEX_PATH_NAME: &str = ".codex"; @@ -805,6 +812,32 @@ impl FileSystemSandboxPolicy { writable_roots: &[AbsolutePathBuf], exclude_tmpdir_env_var: bool, exclude_slash_tmp: bool, + ) -> Self { + let mut policy = Self::workspace_write_template( + writable_roots, + exclude_tmpdir_env_var, + exclude_slash_tmp, + ); + for writable_root in writable_roots { + for protected_path in default_read_only_subpaths_for_writable_root( + writable_root, + /*protect_missing_dot_codex*/ false, + ) { + append_default_read_only_path_if_no_explicit_rule( + &mut policy.entries, + protected_path, + ); + } + } + + policy + } + + /// Builds an unresolved template; admission must bind native workspace metadata. + pub(crate) fn workspace_write_template( + writable_roots: &[AbsolutePathBuf], + exclude_tmpdir_env_var: bool, + exclude_slash_tmp: bool, ) -> Self { let mut entries = vec![FileSystemSandboxEntry::new( FileSystemPath::Special { @@ -846,16 +879,7 @@ impl FileSystemSandboxPolicy { append_default_read_only_project_root_subpath_if_no_explicit_rule(&mut entries, ".agents"); append_default_read_only_project_root_subpath_if_no_explicit_rule(&mut entries, ".codex"); append_default_read_only_project_root_subpath_if_no_explicit_rule(&mut entries, ".aws"); - for writable_root in writable_roots { - for protected_path in default_read_only_subpaths_for_writable_root( - writable_root, - /*protect_missing_dot_codex*/ false, - ) { - append_default_read_only_path_if_no_explicit_rule(&mut entries, protected_path); - } - } - - FileSystemSandboxPolicy::restricted(entries) + Self::restricted(entries) } /// Converts a legacy sandbox policy into an equivalent filesystem policy @@ -1899,6 +1923,17 @@ impl FileSystemSandboxPolicy { &self, network_policy: NetworkSandboxPolicy, cwd: &Path, + ) -> io::Result { + self.to_legacy_sandbox_policy_with_projection( + network_policy, + LegacySandboxPolicyProjection::Cwd(cwd), + ) + } + + pub(crate) fn to_legacy_sandbox_policy_with_projection( + &self, + network_policy: NetworkSandboxPolicy, + projection: LegacySandboxPolicyProjection<'_>, ) -> io::Result { Ok(match self.kind { FileSystemSandboxKind::ExternalSandbox => SandboxPolicy::ExternalSandbox { @@ -1918,7 +1953,12 @@ impl FileSystemSandboxPolicy { } } FileSystemSandboxKind::Restricted => { - let cwd_absolute = AbsolutePathBuf::from_absolute_path(cwd).ok(); + let cwd = match projection { + LegacySandboxPolicyProjection::Cwd(cwd) => Some(cwd), + LegacySandboxPolicyProjection::Template => None, + }; + let cwd_absolute = + cwd.and_then(|cwd| AbsolutePathBuf::from_absolute_path(cwd).ok()); let has_full_disk_write_access = self.has_full_disk_write_access(); let mut workspace_root_writable = false; let mut writable_roots = Vec::new(); @@ -1976,15 +2016,18 @@ impl FileSystemSandboxPolicy { if workspace_root_writable { SandboxPolicy::WorkspaceWrite { - writable_roots: dedup_absolute_paths( - writable_roots, - /*normalize_effective_paths*/ false, - ), + writable_roots: match projection { + LegacySandboxPolicyProjection::Cwd(_) => dedup_absolute_paths( + writable_roots, + /*normalize_effective_paths*/ false, + ), + LegacySandboxPolicyProjection::Template => writable_roots, + }, network_access: network_policy.is_enabled(), exclude_tmpdir_env_var: !tmpdir_writable, exclude_slash_tmp: !slash_tmp_writable, } - } else if self.has_legacy_write_intent(cwd) { + } else if cwd.is_some_and(|cwd| self.has_legacy_write_intent(cwd)) { return Err(io::Error::new( io::ErrorKind::InvalidInput, "permissions profile requests filesystem writes outside the workspace root, which is not supported until the runtime enforces FileSystemSandboxPolicy directly", diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 7a026d04c5..1a79ff504e 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -4115,6 +4115,12 @@ mod tests { missing_path_behavior: None, }); + assert!( + permission_profile + .to_legacy_sandbox_policy_template() + .is_err() + ); + let error = turn_permissions_overrides( TurnPermissionsOverride::LegacySandbox(permission_profile), cwd.as_path(), diff --git a/codex-rs/tui/src/dynamic_tools.rs b/codex-rs/tui/src/dynamic_tools.rs index e06585de15..c294d4a9dd 100644 --- a/codex-rs/tui/src/dynamic_tools.rs +++ b/codex-rs/tui/src/dynamic_tools.rs @@ -22,7 +22,6 @@ use codex_app_server_protocol::ModelListParams; use codex_app_server_protocol::ModelListResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SandboxMode; -use codex_app_server_protocol::SandboxPolicy; use codex_app_server_protocol::SortDirection; use codex_app_server_protocol::Thread; use codex_app_server_protocol::ThreadArchiveParams; @@ -60,6 +59,7 @@ use codex_features::Feature; use codex_features::Features; use codex_protocol::ThreadId; use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::PermissionProfile; use serde::Deserialize; use serde::de::DeserializeOwned; use serde_json::Value; @@ -535,25 +535,38 @@ async fn execute_inner( thread_start_params.runtime_workspace_roots = Some(source.runtime_workspace_roots); thread_start_params.approval_policy = Some(source.approval_policy); thread_start_params.approvals_reviewer = Some(source.approvals_reviewer); - let sandbox_policy = if let Some(profile) = source.active_permission_profile { - thread_start_params.permissions = Some(profile.id); - thread_start_params.sandbox = None; - None - } else { - thread_start_params.permissions = None; - thread_start_params.sandbox = Some(match &source.sandbox { - SandboxPolicy::DangerFullAccess => SandboxMode::DangerFullAccess, - SandboxPolicy::ReadOnly { .. } => SandboxMode::ReadOnly, - SandboxPolicy::WorkspaceWrite { .. } => SandboxMode::WorkspaceWrite, - SandboxPolicy::ExternalSandbox { .. } => { - return Err( - "Cannot inherit an external sandbox without a permission profile" - .to_string(), - ); - } - }); - Some(source.sandbox) - }; + let source_permissions = + PermissionProfile::from_legacy_sandbox_policy_template(&source.sandbox.to_core()); + let permission_profile_override = + if let Some(profile) = source.active_permission_profile { + thread_start_params.permissions = Some(profile.id); + thread_start_params.sandbox = None; + None + } else { + thread_start_params.permissions = None; + thread_start_params.sandbox = Some(match &source_permissions { + PermissionProfile::Disabled => SandboxMode::DangerFullAccess, + PermissionProfile::Managed { .. } => { + if source_permissions + .file_system_sandbox_policy() + .entries + .iter() + .any(|entry| entry.access.can_write()) + { + SandboxMode::WorkspaceWrite + } else { + SandboxMode::ReadOnly + } + } + PermissionProfile::External { .. } => { + return Err( + "Cannot inherit an external sandbox without a permission profile" + .to_string(), + ); + } + }); + Some(source_permissions) + }; if let Some(model) = arguments.model { thread_start_params.model = Some(model); } @@ -597,7 +610,7 @@ async fn execute_inner( "create_thread", prompt, /*model*/ None, - sandbox_policy, + permission_profile_override, cyber_access_program, ) .await?; @@ -735,7 +748,7 @@ async fn execute_inner( "send_message_to_thread", prompt, arguments.model, - /*sandbox_policy*/ None, + /*permission_profile_override*/ None, cyber_access_program, ) .await?; @@ -1243,9 +1256,13 @@ async fn start_turn( tool: &str, prompt: String, model: Option, - sandbox_policy: Option, + permission_profile_override: Option, cyber_access_program: Option, ) -> Result { + let sandbox_policy = permission_profile_override + .map(|profile| profile.to_legacy_sandbox_policy_template().map(Into::into)) + .transpose() + .map_err(|error| error.to_string())?; request(handle, |request_id| ClientRequest::TurnStart { request_id, params: TurnStartParams { diff --git a/codex-rs/tui/src/dynamic_tools_tests.rs b/codex-rs/tui/src/dynamic_tools_tests.rs index 5190b56410..9da6e1295a 100644 --- a/codex-rs/tui/src/dynamic_tools_tests.rs +++ b/codex-rs/tui/src/dynamic_tools_tests.rs @@ -5,6 +5,8 @@ use crate::legacy_core::config::ConfigBuilder; use app_test_support::create_fake_paginated_rollout; use app_test_support::create_fake_rollout; use app_test_support::rollout_path; +use codex_app_server_client::AppServerEvent; +use codex_app_server_protocol::ServerNotification; use codex_protocol::ThreadId; use core_test_support::responses; use pretty_assertions::assert_eq; @@ -707,7 +709,7 @@ async fn wait_threads_preserves_snapshots_and_rejects_self_wait() -> color_eyre: #[tokio::test] async fn task_creation_and_followup_start_background_turns() -> color_eyre::Result<()> { - let (_codex_home, server, source, target) = test_server(json!({})).await?; + let (codex_home, mut server, source, target) = test_server(json!({})).await?; for (tool, arguments) in [ ( @@ -753,6 +755,36 @@ async fn task_creation_and_followup_start_background_turns() -> color_eyre::Resu .await; assert!(!rejected.success); + let inherited_sandbox = codex_app_server_protocol::SandboxPolicy::WorkspaceWrite { + writable_roots: vec![ + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(codex_home.path())?, + ], + network_access: true, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + request::( + &server.request_handle(), + |request_id| ClientRequest::ThreadSettingsUpdate { + request_id, + params: codex_app_server_protocol::ThreadSettingsUpdateParams { + thread_id: target.clone(), + sandbox_policy: Some(inherited_sandbox.clone()), + ..Default::default() + }, + }, + ) + .await + .map_err(color_eyre::eyre::Error::msg)?; + while let Some(event) = server.next_event().await { + if let AppServerEvent::ServerNotification(notification) = event.event + && matches!(*notification, ServerNotification::ThreadSettingsUpdated(ref update) + if update.thread_id == target && update.thread_settings.sandbox_policy == inherited_sandbox) + { + break; + } + } + let created = response_json( call_tool( &server, @@ -763,6 +795,21 @@ async fn task_creation_and_followup_start_background_turns() -> color_eyre::Resu .await, ); assert!(created["threadId"].is_string()); + let child: ThreadResumeResponse = request(&server.request_handle(), |request_id| { + ClientRequest::ThreadResume { + request_id, + params: ThreadResumeParams { + thread_id: created["threadId"] + .as_str() + .expect("created thread id") + .to_string(), + ..Default::default() + }, + } + }) + .await + .map_err(color_eyre::eyre::Error::msg)?; + assert_eq!(child.sandbox, inherited_sandbox); let continued = response_json( call_tool( diff --git a/codex-rs/tui/src/temporary_structured_request.rs b/codex-rs/tui/src/temporary_structured_request.rs index b19590422c..bea02620aa 100644 --- a/codex-rs/tui/src/temporary_structured_request.rs +++ b/codex-rs/tui/src/temporary_structured_request.rs @@ -5,7 +5,6 @@ use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::SandboxMode; -use codex_app_server_protocol::SandboxPolicy; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ThreadItemPayload; use codex_app_server_protocol::ThreadSource; @@ -20,6 +19,7 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY; +use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; use color_eyre::eyre::eyre; use serde_json::Value; @@ -163,11 +163,21 @@ pub(crate) async fn start_temporary_thread( "temporary structured thread did not preserve permission profile {expected_profile}" )); } - } else if !matches!(response.sandbox, SandboxPolicy::ReadOnly { .. }) { - unsubscribe_temporary_thread(request_handle, response.thread.id).await; - return Err(eyre!( - "temporary structured thread did not start with read-only permissions" - )); + } else { + let profile = + PermissionProfile::from_legacy_sandbox_policy_template(&response.sandbox.to_core()); + if !matches!(profile, PermissionProfile::Managed { .. }) + || profile + .file_system_sandbox_policy() + .entries + .iter() + .any(|entry| entry.access.can_write()) + { + unsubscribe_temporary_thread(request_handle, response.thread.id).await; + return Err(eyre!( + "temporary structured thread did not start with read-only permissions" + )); + } } Ok(response)