diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index ad8adf14cb..100b7cdcfd 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -233,17 +233,17 @@ pub enum GuardianApprovalRequestSource { pub enum GuardianReviewedAction { Shell { sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option>, }, UnifiedExec { sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option>, tty: bool, }, Execve { source: GuardianCommandSource, program: String, - additional_permissions: Option, + additional_permissions: Option>, }, ApplyPatch {}, NetworkAccess { diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 4cba4bddc5..c1540c2664 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -25,6 +25,7 @@ use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputArgs; use codex_protocol::request_user_input::RequestUserInputResponse; use codex_protocol::user_input::UserInput; +use codex_utils_path_uri::PathUri; use serde_json::Value; use std::time::Duration; use tokio::sync::Mutex; @@ -590,66 +591,97 @@ async fn handle_patch_approval( } = event; let approval_id = call_id.clone(); let guardian_decision = if routes_approval_to_guardian(parent_ctx) { - let files = changes - .keys() - .map(|path| { - #[allow(deprecated)] - parent_ctx.cwd.join(path) - }) - .collect::>(); - let review_cancel = cancel_token.child_token(); - let patch = changes - .iter() - .map(|(path, change)| match change { - codex_protocol::protocol::FileChange::Add { content } => { - format!("*** Add File: {}\n{}", path.display(), content) - } - codex_protocol::protocol::FileChange::Delete { content } => { - format!("*** Delete File: {}\n{}", path.display(), content) - } - codex_protocol::protocol::FileChange::Update { - unified_diff, - move_path, - } => { - if let Some(move_path) = move_path { - format!( - "*** Update File: {}\n*** Move to: {}\n{}", - path.display(), - move_path.display(), - unified_diff + match parent_ctx.environments.primary() { + Some(environment) => { + let cwd = environment.cwd().clone(); + let files = changes + .iter() + .flat_map(|(path, change)| { + std::iter::once(path).chain(match change { + codex_protocol::protocol::FileChange::Update { + move_path: Some(move_path), + .. + } => Some(move_path), + codex_protocol::protocol::FileChange::Add { .. } + | codex_protocol::protocol::FileChange::Delete { .. } + | codex_protocol::protocol::FileChange::Update { + move_path: None, + .. + } => None, + }) + }) + .map(|path| cwd.join(path.to_string_lossy().as_ref())) + .collect::, _>>(); + match files { + Ok(files) => { + let review_cancel = cancel_token.child_token(); + let patch = changes + .iter() + .map(|(path, change)| match change { + codex_protocol::protocol::FileChange::Add { content } => { + format!("*** Add File: {}\n{}", path.display(), content) + } + codex_protocol::protocol::FileChange::Delete { content } => { + format!("*** Delete File: {}\n{}", path.display(), content) + } + codex_protocol::protocol::FileChange::Update { + unified_diff, + move_path, + } => { + if let Some(move_path) = move_path { + format!( + "*** Update File: {}\n*** Move to: {}\n{}", + path.display(), + move_path.display(), + unified_diff + ) + } else { + format!( + "*** Update File: {}\n{}", + path.display(), + unified_diff + ) + } + } + }) + .collect::>() + .join("\n"); + let review_rx = spawn_approval_request_review( + Arc::clone(parent_session), + Arc::clone(parent_ctx), + new_guardian_review_id(), + GuardianApprovalRequest::ApplyPatch { + id: approval_id.clone(), + cwd, + files, + patch, + }, + reason.clone(), + GuardianApprovalRequestSource::DelegatedSubagent, + review_cancel.clone(), + ); + Some( + await_approval_with_cancel( + async move { review_rx.await.unwrap_or_default() }, + parent_session, + &approval_id, + cancel_token, + Some(&review_cancel), + ) + .await, ) - } else { - format!("*** Update File: {}\n{}", path.display(), unified_diff) + } + Err(error) => { + tracing::warn!(%error, %cwd, "failed to resolve delegated patch path"); + Some(ReviewDecision::Abort) } } - }) - .collect::>() - .join("\n"); - let review_rx = spawn_approval_request_review( - Arc::clone(parent_session), - Arc::clone(parent_ctx), - new_guardian_review_id(), - GuardianApprovalRequest::ApplyPatch { - id: approval_id.clone(), - #[allow(deprecated)] - cwd: parent_ctx.cwd.clone(), - files, - patch, - }, - reason.clone(), - GuardianApprovalRequestSource::DelegatedSubagent, - review_cancel.clone(), - ); - Some( - await_approval_with_cancel( - async move { review_rx.await.unwrap_or_default() }, - parent_session, - &approval_id, - cancel_token, - Some(&review_cancel), - ) - .await, - ) + } + None => { + tracing::warn!("delegated patch approval has no primary environment"); + Some(ReviewDecision::Abort) + } + } } else { None }; @@ -806,10 +838,18 @@ async fn handle_request_permissions( reason: event.reason, permissions: event.permissions, }; - let cwd = event.cwd.unwrap_or_else(|| { - #[allow(deprecated)] - parent_ctx.cwd.clone() - }); + let cwd = event + .cwd + .or_else(|| { + parent_ctx + .environments + .primary() + .map(|environment| environment.cwd().clone()) + }) + .unwrap_or_else(|| { + #[allow(deprecated)] + PathUri::from_abs_path(&parent_ctx.cwd) + }); let response_fut = parent_session.request_permissions_for_cwd( parent_ctx, call_id.clone(), @@ -859,9 +899,9 @@ async fn await_request_permissions_with_cancel( parent_session: &Session, call_id: &str, cancel_token: &CancellationToken, -) -> RequestPermissionsResponse +) -> RequestPermissionsResponse where - F: core::future::Future>, + F: core::future::Future>>, { tokio::select! { biased; diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 924750bc34..d9e1159147 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -48,6 +48,7 @@ use codex_protocol::protocol::TurnAbortReason; use codex_protocol::protocol::WarningEvent; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::request_user_input::RequestUserInputResponse; +use codex_utils_path_uri::PathUri; use crate::context_manager::is_user_turn_boundary; use codex_protocol::dynamic_tools::DynamicToolResponse; @@ -423,7 +424,7 @@ pub async fn request_user_input_response( pub async fn request_permissions_response( sess: &Arc, id: String, - response: RequestPermissionsResponse, + response: RequestPermissionsResponse, ) { sess.notify_request_permissions_response(&id, response) .await; diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index b01567f668..d09ecd1926 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2615,10 +2615,9 @@ impl Session { } PermissionGrantScope::Session => { let mut state = self.state.lock().await; - state.record_granted_permissions( - environment_id, - response.permissions.clone().into(), - ); + let permissions: AdditionalPermissionProfile = + response.permissions.clone().into(); + state.record_granted_permissions(environment_id, permissions); } } } diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index b9bf42073b..935193e975 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -12,8 +12,7 @@ use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; -use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; -use codex_sandboxing::policy_transforms::effective_network_sandbox_policy; +use codex_sandboxing::policy_transforms::effective_permission_profile_for_uri; use codex_utils_path_uri::PathUri; use futures::FutureExt; use futures::future::BoxFuture; @@ -298,33 +297,31 @@ impl TurnContext { pub(crate) fn file_system_sandbox_context( &self, - additional_permissions: Option, - cwd: &PathUri, + additional_permissions: Option>, + environment: &TurnEnvironment, ) -> FileSystemSandboxContext { - let (base_file_system_sandbox_policy, base_network_sandbox_policy) = - self.permission_profile.to_runtime_permissions(); - let file_system_sandbox_policy = effective_file_system_sandbox_policy( - &base_file_system_sandbox_policy, + let environment_is_remote = environment.environment.is_remote(); + let permission_profile = if environment_is_remote { + self.config.permissions.permission_profile() + } else { + &self.permission_profile + }; + let permissions = effective_permission_profile_for_uri( + permission_profile, additional_permissions.as_ref(), ); - let network_sandbox_policy = effective_network_sandbox_policy( - base_network_sandbox_policy, - additional_permissions.as_ref(), - ); - let permissions = PermissionProfile::from_runtime_permissions_with_enforcement( - self.permission_profile.enforcement(), - &file_system_sandbox_policy, - network_sandbox_policy, - ); FileSystemSandboxContext { - permissions: permissions.into(), - cwd: Some(cwd.clone()), - workspace_roots: self - .config - .effective_workspace_roots() - .iter() - .map(PathUri::from_abs_path) - .collect(), + permissions, + cwd: Some(environment.cwd().clone()), + workspace_roots: if environment_is_remote { + Vec::new() + } else { + self.config + .effective_workspace_roots() + .iter() + .map(PathUri::from_abs_path) + .collect() + }, windows_sandbox_level: self.windows_sandbox_level, windows_sandbox_private_desktop: self .config diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index d681c6fa87..8c8d281217 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -3,6 +3,7 @@ use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::models::ResponseItem; use codex_sandboxing::policy_transforms::merge_permission_profiles; +use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -41,7 +42,7 @@ pub(crate) struct SessionState { pub(crate) current_time_reminder: CurrentTimeReminderState, pub(crate) active_connector_selection: HashSet, pub(crate) pending_session_start_sources: VecDeque, - granted_permissions_by_environment_id: HashMap, + granted_permissions_by_environment_id: HashMap>, next_turn_is_first: bool, } @@ -288,7 +289,7 @@ impl SessionState { pub(crate) fn record_granted_permissions( &mut self, environment_id: &str, - permissions: AdditionalPermissionProfile, + permissions: AdditionalPermissionProfile, ) { let granted_permissions = merge_permission_profiles( self.granted_permissions_by_environment_id @@ -304,7 +305,7 @@ impl SessionState { pub(crate) fn granted_permissions( &self, environment_id: &str, - ) -> Option { + ) -> Option> { self.granted_permissions_by_environment_id .get(environment_id) .cloned() diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 6326dbc1ad..7155d5928b 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -1,4 +1,3 @@ -use std::collections::BTreeSet; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -46,12 +45,10 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::FileChange; use codex_protocol::protocol::PatchApplyUpdatedEvent; -use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; -use codex_sandboxing::policy_transforms::merge_permission_profiles; -use codex_sandboxing::policy_transforms::normalize_additional_permissions; +use codex_sandboxing::policy_transforms::normalize_additional_permissions_for_uri; use codex_tools::ToolName; use codex_tools::ToolSpec; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500); @@ -219,25 +216,37 @@ fn file_paths_for_action(action: &ApplyPatchAction) -> Vec { } fn write_permissions_for_paths( - file_paths: &[AbsolutePathBuf], + file_paths: &[PathUri], file_system_sandbox_policy: &codex_protocol::permissions::FileSystemSandboxPolicy, - cwd: &AbsolutePathBuf, -) -> Option { - let write_paths = file_paths - .iter() - .map(|path| { - path.parent() - .unwrap_or_else(|| path.clone()) - .into_path_buf() - }) - .filter(|path| { - !file_system_sandbox_policy.can_write_path_with_cwd(path.as_path(), cwd.as_path()) - }) - .collect::>() - .into_iter() - .map(AbsolutePathBuf::from_absolute_path) - .collect::, _>>() - .ok()?; + cwd: &PathUri, +) -> Option> { + let native_cwd = if cwd.infer_path_convention() == Some(PathConvention::native()) { + cwd.to_abs_path().ok() + } else { + None + }; + let mut write_paths = Vec::new(); + for file_path in file_paths { + let already_writable = native_cwd.as_ref().is_some_and(|native_cwd| { + file_path.infer_path_convention() == Some(PathConvention::native()) + && file_path + .to_abs_path() + .ok() + .is_some_and(|native_file_path| { + file_system_sandbox_policy.can_write_path_with_cwd( + native_file_path.as_path(), + native_cwd.as_path(), + ) + }) + }); + if already_writable { + continue; + } + let write_path = file_path.parent().unwrap_or_else(|| file_path.clone()); + if !write_paths.contains(&write_path) { + write_paths.push(write_path); + } + } let permissions = (!write_paths.is_empty()).then_some(AdditionalPermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( @@ -247,7 +256,7 @@ fn write_permissions_for_paths( ..Default::default() })?; - normalize_additional_permissions(permissions).ok() + normalize_additional_permissions_for_uri(permissions).ok() } /// Extracts the raw patch text used as the command-shaped hook input for apply_patch. @@ -264,65 +273,26 @@ async fn effective_patch_permissions( environment_id: &str, action: &ApplyPatchAction, cwd: &PathUri, -) -> std::io::Result<( - Vec, - crate::tools::handlers::EffectiveAdditionalPermissions, - codex_protocol::permissions::FileSystemSandboxPolicy, -)> { - let file_paths = file_paths_for_action(action); - let native_cwd = cwd.to_abs_path()?; - let granted_permissions = merge_permission_profiles( - session - .granted_session_permissions(environment_id) - .await - .as_ref(), - session - .granted_turn_permissions(environment_id) - .await - .as_ref(), - ); - let base_file_system_sandbox_policy = turn.file_system_sandbox_policy(); - let file_system_sandbox_policy = effective_file_system_sandbox_policy( - &base_file_system_sandbox_policy, - granted_permissions.as_ref(), - ); - let native_file_paths = file_paths - .iter() - .map(PathUri::to_abs_path) - .collect::, _>>()?; - let effective_additional_permissions = apply_granted_turn_permissions( - session, - environment_id, - native_cwd.as_path(), - crate::sandboxing::SandboxPermissions::UseDefault, - write_permissions_for_paths(&native_file_paths, &file_system_sandbox_policy, &native_cwd), - ) - .await; - - Ok(( - file_paths, - effective_additional_permissions, - file_system_sandbox_policy, - )) -} - -fn patch_permissions_without_path_matching( - action: &ApplyPatchAction, ) -> ( Vec, crate::tools::handlers::EffectiveAdditionalPermissions, codex_protocol::permissions::FileSystemSandboxPolicy, ) { - // TODO(anp): Make permission matching operate on PathUri. Until then, foreign paths skip - // permission matching; a managed turn still fails closed at the platform sandbox boundary. + let file_paths = file_paths_for_action(action); + let file_system_sandbox_policy = turn.file_system_sandbox_policy(); + let effective_additional_permissions = apply_granted_turn_permissions( + session, + environment_id, + cwd, + crate::sandboxing::SandboxPermissions::UseDefault, + write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, cwd), + ) + .await; + ( - file_paths_for_action(action), - crate::tools::handlers::EffectiveAdditionalPermissions { - sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, - additional_permissions: None, - permissions_preapproved: false, - }, - codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted(), + file_paths, + effective_additional_permissions, + file_system_sandbox_policy, ) } @@ -383,10 +353,8 @@ impl ApplyPatchHandler { )); }; let fs = turn_environment.environment.get_filesystem(); - let sandbox = turn.file_system_sandbox_context( - /*additional_permissions*/ None, - turn_environment.cwd(), - ); + let sandbox = turn + .file_system_sandbox_context(/*additional_permissions*/ None, turn_environment); match codex_apply_patch::verify_apply_patch_args( args, turn_environment.cwd(), @@ -404,8 +372,7 @@ impl ApplyPatchHandler { &changes, turn_environment.cwd(), ) - .await - .unwrap_or_else(|_| patch_permissions_without_path_matching(&changes)); + .await; match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes) .await { @@ -554,7 +521,8 @@ pub(crate) async fn intercept_apply_patch( call_id: &str, tool_name: &str, ) -> Result, FunctionCallError> { - let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, cwd); + let sandbox = + turn.file_system_sandbox_context(/*additional_permissions*/ None, &turn_environment); match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, Some(&sandbox)) .await { @@ -567,8 +535,7 @@ pub(crate) async fn intercept_apply_patch( &changes, cwd, ) - .await - .unwrap_or_else(|_| patch_permissions_without_path_matching(&changes)); + .await; match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes) .await { diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 2fd998849e..edeffd3e62 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -117,15 +117,13 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { let mut environments = Vec::with_capacity(invocation.step_context.environments.turn_environments.len()); for environment in &invocation.step_context.environments.turn_environments { - // TODO(anp): Migrate extension ToolEnvironment and granted-permission lookup to PathUri - // so extensions can receive foreign environment cwd values. let Ok(native_cwd) = environment.cwd().to_abs_path() else { continue; }; let additional_permissions = apply_granted_turn_permissions( invocation.session.as_ref(), &environment.environment_id, - native_cwd.as_path(), + environment.cwd(), SandboxPermissions::UseDefault, /*additional_permissions*/ None, ) @@ -133,7 +131,7 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { .additional_permissions; let file_system_sandbox_context = invocation .turn - .file_system_sandbox_context(additional_permissions, environment.cwd()); + .file_system_sandbox_context(additional_permissions, environment); environments.push(ToolEnvironment { environment_id: environment.environment_id.clone(), cwd: native_cwd, diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 29ad172979..574cc2e090 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -37,15 +37,15 @@ mod view_image; pub(crate) mod view_image_spec; mod wait_for_environment; -use codex_sandboxing::policy_transforms::intersect_permission_profiles; +use codex_sandboxing::policy_transforms::intersect_permission_profiles_for_uri; use codex_sandboxing::policy_transforms::merge_permission_profiles; -use codex_sandboxing::policy_transforms::normalize_additional_permissions; +use codex_sandboxing::policy_transforms::normalize_additional_permissions_for_uri; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use serde::Deserialize; use serde_json::Map; use serde_json::Value; -use std::path::Path; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::function_tool::FunctionCallError; @@ -181,10 +181,10 @@ pub(crate) fn normalize_and_validate_additional_permissions( additional_permissions_allowed: bool, approval_policy: AskForApproval, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option>, permissions_preapproved: bool, - _cwd: &Path, -) -> Result, String> { + _cwd: &PathUri, +) -> Result>, String> { let uses_additional_permissions = matches!( sandbox_permissions, SandboxPermissions::WithAdditionalPermissions @@ -212,7 +212,7 @@ pub(crate) fn normalize_and_validate_additional_permissions( .to_string(), ); }; - let normalized = normalize_additional_permissions(additional_permissions)?; + let normalized = normalize_additional_permissions_for_uri(additional_permissions)?; if normalized.is_empty() { return Err( "`additional_permissions` must include at least one requested permission in `network` or `file_system`" @@ -234,15 +234,15 @@ pub(crate) fn normalize_and_validate_additional_permissions( pub(super) struct EffectiveAdditionalPermissions { pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option>, pub permissions_preapproved: bool, } pub(super) fn implicit_granted_permissions( sandbox_permissions: SandboxPermissions, - additional_permissions: Option<&AdditionalPermissionProfile>, + additional_permissions: Option<&AdditionalPermissionProfile>, effective_additional_permissions: &EffectiveAdditionalPermissions, -) -> Option { +) -> Option> { if !sandbox_permissions.uses_additional_permissions() && !matches!(sandbox_permissions, SandboxPermissions::RequireEscalated) && additional_permissions.is_none() @@ -258,9 +258,9 @@ pub(super) fn implicit_granted_permissions( pub(super) async fn apply_granted_turn_permissions( session: &Session, environment_id: &str, - cwd: &Path, + cwd: &PathUri, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option>, ) -> EffectiveAdditionalPermissions { if matches!(sandbox_permissions, SandboxPermissions::RequireEscalated) { return EffectiveAdditionalPermissions { @@ -302,16 +302,16 @@ pub(super) async fn apply_granted_turn_permissions( } fn permissions_are_preapproved( - effective_permissions: &AdditionalPermissionProfile, - granted_permissions: AdditionalPermissionProfile, - cwd: &Path, + effective_permissions: &AdditionalPermissionProfile, + granted_permissions: AdditionalPermissionProfile, + cwd: &PathUri, ) -> bool { - let materialized_effective_permissions = intersect_permission_profiles( + let materialized_effective_permissions = intersect_permission_profiles_for_uri( effective_permissions.clone(), effective_permissions.clone(), cwd, ); - intersect_permission_profiles(effective_permissions.clone(), granted_permissions, cwd) + intersect_permission_profiles_for_uri(effective_permissions.clone(), granted_permissions, cwd) == materialized_effective_permissions } @@ -331,13 +331,13 @@ mod tests { use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::GranularApprovalConfig; - use codex_sandboxing::policy_transforms::intersect_permission_profiles; + use codex_sandboxing::policy_transforms::intersect_permission_profiles_for_uri; use codex_sandboxing::policy_transforms::merge_permission_profiles; - use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tempfile::tempdir; - fn network_permissions() -> AdditionalPermissionProfile { + fn network_permissions() -> AdditionalPermissionProfile { AdditionalPermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), @@ -346,12 +346,12 @@ mod tests { } } - fn file_system_permissions(path: &std::path::Path) -> AdditionalPermissionProfile { + fn file_system_permissions(path: &std::path::Path) -> AdditionalPermissionProfile { AdditionalPermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( /*read*/ None, Some(vec![ - AbsolutePathBuf::from_absolute_path(path).expect("absolute path"), + PathUri::from_host_native_path(path).expect("absolute path URI"), ]), )), ..Default::default() @@ -362,6 +362,7 @@ mod tests { fn preapproved_permissions_work_when_request_permissions_tool_is_enabled_without_exec_permission_approvals_feature() { let cwd = tempdir().expect("tempdir"); + let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI"); let normalized = normalize_and_validate_additional_permissions( /*additional_permissions_allowed*/ false, @@ -375,7 +376,7 @@ mod tests { SandboxPermissions::WithAdditionalPermissions, Some(network_permissions()), /*permissions_preapproved*/ true, - cwd.path(), + &cwd_uri, ) .expect("preapproved permissions should be allowed"); @@ -385,6 +386,7 @@ mod tests { #[test] fn fresh_additional_permissions_still_require_exec_permission_approvals_feature() { let cwd = tempdir().expect("tempdir"); + let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI"); let err = normalize_and_validate_additional_permissions( /*additional_permissions_allowed*/ false, @@ -392,7 +394,7 @@ mod tests { SandboxPermissions::WithAdditionalPermissions, Some(network_permissions()), /*permissions_preapproved*/ false, - cwd.path(), + &cwd_uri, ) .expect_err("fresh inline permission requests should remain disabled"); @@ -439,6 +441,7 @@ mod tests { #[test] fn relative_deny_glob_grants_remain_preapproved_after_materialization() { let cwd = tempdir().expect("tempdir"); + let cwd_uri = PathUri::from_host_native_path(cwd.path()).expect("cwd URI"); let requested_permissions = AdditionalPermissionProfile { file_system: Some(FileSystemPermissions { entries: vec![ @@ -459,10 +462,10 @@ mod tests { }), ..Default::default() }; - let stored_grant = intersect_permission_profiles( + let stored_grant = intersect_permission_profiles_for_uri( requested_permissions.clone(), requested_permissions.clone(), - cwd.path(), + &cwd_uri, ); let effective_permissions = merge_permission_profiles(Some(&requested_permissions), Some(&stored_grant)) @@ -471,7 +474,7 @@ mod tests { assert!(permissions_are_preapproved( &effective_permissions, stored_grant, - cwd.path(), + &cwd_uri, )); } } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 87a9301093..7b0cdb264b 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -87,13 +87,14 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result Result Result, + additional_permissions: Option>, #[serde(default)] justification: Option, #[serde(default)] diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index 546ad459e1..e65d498dff 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -12,7 +12,6 @@ use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::handlers::implicit_granted_permissions; use crate::tools::handlers::normalize_and_validate_additional_permissions; use crate::tools::handlers::parse_arguments; -use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::rewrite_function_string_argument; use crate::tools::handlers::updated_hook_command; @@ -29,9 +28,7 @@ use crate::unified_exec::generate_chunk_id; use codex_features::Feature; use codex_otel::SessionTelemetry; use codex_otel::TOOL_CALL_UNIFIED_EXEC_METRIC; -use codex_sandboxing::SandboxManager; -use codex_sandboxing::SandboxType; -use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::policy_transforms::resolve_additional_permission_paths; use codex_shell_command::shell_detect::detect_shell_type; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -151,52 +148,23 @@ impl ExecCommandHandler { let environment = Arc::clone(&turn_environment.environment); let fs = environment.get_filesystem(); - // A foreign cwd cannot seed the AbsolutePathBufGuard used to resolve relative paths in the - // permissions config below. Consult the configured platform-sandbox requirement before - // deciding whether parsing may continue without that base path. - let sandbox = SandboxManager::new().select_initial( - &turn.file_system_sandbox_policy(), - turn.network_sandbox_policy(), - SandboxablePreference::Auto, - turn.windows_sandbox_level, - turn.network.is_some(), - ); - // `to_abs_path()` alone cannot identify foreign drive paths: `file:///C:/repo` is - // representable as `/C:/repo` on POSIX. Require the inferred convention to match too. - let cwd_uses_native_convention = - cwd.infer_path_convention() == Some(PathConvention::native()); - // TODO(anp): Remove this parsing split once sandboxing supports foreign paths. - let native_cwd = match cwd.to_abs_path() { - Ok(cwd) if cwd_uses_native_convention => Some(cwd), - _ if sandbox == SandboxType::None => None, - Err(err) => return Err(FunctionCallError::RespondToModel(err.to_string())), - Ok(_) => { - return Err(FunctionCallError::RespondToModel(format!( - "path URI `{cwd}` does not use the host's native {} path convention", - PathConvention::native() - ))); - } - }; - let mut args: ExecCommandArgs = match native_cwd.as_ref() { - Some(native_cwd) => { - // The base path only resolves paths nested in the permissions config types. - parse_arguments_with_base_path(&arguments, native_cwd)? - } - None => { - // Parsing without a base only skips relative-path resolution inside the - // permissions config. That is safe only for a truly unsandboxed attempt; - // sandboxed attempts fall through and return the conversion error below. - parse_arguments(&arguments)? - } - }; + let mut args: ExecCommandArgs = parse_arguments(&arguments)?; + let additional_permissions = args + .additional_permissions + .take() + .map(|permissions| resolve_additional_permission_paths(permissions, &cwd)) + .transpose() + .map_err(FunctionCallError::RespondToModel)?; let hook_command = args.cmd.clone(); - // TODO(anp) wire PathUri through implicit skills instead of skipping on foreign paths - if let Some(native_cwd) = native_cwd.as_ref() { + // TODO(anp): Wire PathUri through implicit skills instead of skipping foreign paths. + if cwd.infer_path_convention() == Some(PathConvention::native()) + && let Ok(native_cwd) = cwd.to_abs_path() + { maybe_emit_implicit_skill_invocation( session.as_ref(), context.turn.as_ref(), &hook_command, - native_cwd, + &native_cwd, ) .await; } @@ -245,7 +213,6 @@ impl ExecCommandHandler { yield_time_ms, max_output_tokens, sandbox_permissions, - additional_permissions, justification, prefix_rule, .. @@ -254,12 +221,10 @@ impl ExecCommandHandler { let exec_permission_approvals_enabled = session.features().enabled(Feature::ExecPermissionApprovals); let requested_additional_permissions = additional_permissions.clone(); - // TODO(anp): Make permission matching operate on PathUri for remote environments. - let permission_cwd = native_cwd.as_ref().unwrap_or(&turn.config.cwd); let effective_additional_permissions = apply_granted_turn_permissions( context.session.as_ref(), &turn_environment.environment_id, - permission_cwd.as_path(), + &cwd, sandbox_permissions, additional_permissions, ) @@ -299,7 +264,7 @@ impl ExecCommandHandler { effective_additional_permissions.sandbox_permissions, effective_additional_permissions.additional_permissions, effective_additional_permissions.permissions_preapproved, - permission_cwd, + &cwd, ) }, |permissions| Ok(Some(permissions)), diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 625b5b8713..c3ec34ed22 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -147,10 +147,8 @@ impl ViewImageHandler { )) })?; let model_visible_path = path_uri.inferred_native_path_string(); - let sandbox = turn.file_system_sandbox_context( - /*additional_permissions*/ None, - turn_environment.cwd(), - ); + let sandbox = turn + .file_system_sandbox_context(/*additional_permissions*/ None, turn_environment); let fs = turn_environment.environment.get_filesystem(); let metadata = fs diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 747ae80a6f..4409681228 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -28,6 +28,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::WarningEvent; use codex_sandboxing::SandboxType; +use codex_utils_path_uri::PathUri; use indexmap::IndexMap; use std::collections::HashMap; use std::collections::HashSet; @@ -652,19 +653,23 @@ impl NetworkApprovalService { .await } else { let available_decisions = None; - let cwd = if let Some(owner_call) = owner_call.as_ref() { - owner_call.trigger.cwd.clone() + let cwd_uri = if let Some(owner_call) = owner_call.as_ref() { + Some(owner_call.trigger.cwd.clone()) } else { turn_context .environments .turn_environments .iter() .find(|environment| environment.environment_id == environment_id) - .and_then(|environment| environment.cwd().to_abs_path().ok()) - .unwrap_or_else(|| { - #[allow(deprecated)] - turn_context.cwd.clone() - }) + .map(|environment| environment.cwd().clone()) + }; + let cwd = match cwd_uri { + Some(cwd) => cwd, + None => + { + #[allow(deprecated)] + PathUri::from_abs_path(&turn_context.cwd) + } }; session .request_command_approval( diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index b839b05c44..c0db967832 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -30,7 +30,7 @@ use codex_protocol::protocol::FileChange; use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; -use codex_sandboxing::policy_transforms::effective_permission_profile; +use codex_sandboxing::policy_transforms::effective_permission_profile_for_uri; use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use std::path::PathBuf; @@ -49,7 +49,7 @@ pub struct ApplyPatchRequest { pub file_paths: Vec, pub changes: std::collections::HashMap, pub exec_approval_requirement: ExecApprovalRequirement, - pub additional_permissions: Option, + pub additional_permissions: Option>, pub permissions_preapproved: bool, } @@ -77,17 +77,10 @@ impl ApplyPatchRuntime { req: &ApplyPatchRequest, call_id: &str, ) -> std::io::Result { - // TODO(anp): Remove this conversion once the guardian API supports PathUri. - let cwd = req.action.cwd.to_abs_path()?; - let files = req - .file_paths - .iter() - .map(PathUri::to_abs_path) - .collect::>>()?; Ok(ApprovalAction::ApplyPatch { id: call_id.to_string(), - cwd, - files, + cwd: req.action.cwd.clone(), + files: req.file_paths.clone(), patch: req.action.patch.clone(), }) } @@ -100,16 +93,28 @@ impl ApplyPatchRuntime { return None; } - let permissions = - effective_permission_profile(attempt.permissions, req.additional_permissions.as_ref()); + let environment_is_remote = req.turn_environment.environment.is_remote(); + let permission_profile = if environment_is_remote { + attempt.exec_server_permissions + } else { + attempt.permissions + }; + let permissions = effective_permission_profile_for_uri( + permission_profile, + req.additional_permissions.as_ref(), + ); Some(FileSystemSandboxContext { - permissions: permissions.into(), + permissions, cwd: Some(attempt.sandbox_cwd.clone()), - workspace_roots: attempt - .workspace_roots - .iter() - .map(PathUri::from_abs_path) - .collect(), + workspace_roots: if environment_is_remote { + Vec::new() + } else { + attempt + .workspace_roots + .iter() + .map(PathUri::from_abs_path) + .collect() + }, windows_sandbox_level: attempt.windows_sandbox_level, windows_sandbox_private_desktop: attempt.windows_sandbox_private_desktop, use_legacy_landlock: attempt.use_legacy_landlock, diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 1a83ef5657..f6e460ec40 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -40,7 +40,7 @@ pub(crate) fn build_sandbox_command( command: &[String], cwd: &AbsolutePathBuf, env: &HashMap, - additional_permissions: Option, + additional_permissions: Option>, ) -> Result { let (program, args) = command .split_first() diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 3ff50f3f51..755a1f42fe 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -46,6 +46,7 @@ use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxablePreference; use codex_shell_command::powershell::prefix_powershell_script_with_utf8; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use std::collections::HashMap; use tokio_util::sync::CancellationToken; @@ -63,7 +64,7 @@ pub struct ShellRequest { pub explicit_env_overrides: HashMap, pub network: Option, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option>, #[cfg(unix)] pub additional_permissions_preapproved: bool, pub justification: Option, @@ -96,7 +97,7 @@ pub(crate) struct ApprovalKey { command: Vec, cwd: AbsolutePathBuf, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option>, } impl ShellRuntime { @@ -142,7 +143,7 @@ impl Approvable for ShellRuntime { ) -> BoxFuture<'a, ReviewDecision> { let keys = self.approval_keys(req); let command = req.command.clone(); - let cwd = req.cwd.clone(); + let cwd = PathUri::from_abs_path(&req.cwd); let environment_id = Some(req.turn_environment.environment_id.clone()); let reason = ctx .retry_reason @@ -184,7 +185,7 @@ impl Approvable for ShellRuntime { Ok(ApprovalAction::Shell { id: ctx.call_id.to_string(), command: req.command.clone(), - cwd: req.cwd.clone(), + cwd: PathUri::from_abs_path(&req.cwd), sandbox_permissions: req.sandbox_permissions, additional_permissions: req.additional_permissions.clone(), justification: req.justification.clone(), @@ -227,7 +228,7 @@ impl ToolRuntime for ShellRuntime { call_id: ctx.call_id.clone(), tool_name: flat_tool_name(&ctx.tool_name).into_owned(), command: req.command.clone(), - cwd: req.cwd.clone(), + cwd: PathUri::from_abs_path(&req.cwd), sandbox_permissions: req.sandbox_permissions, additional_permissions: req.additional_permissions.clone(), justification: req.justification.clone(), diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 10cb460205..962b91b6fc 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -49,6 +49,7 @@ use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; +use codex_sandboxing::localize_additional_permissions_for_current_host; use codex_shell_command::bash::parse_shell_lc_plain_commands; use codex_shell_command::bash::parse_shell_lc_single_command_prefix; use codex_shell_escalation::EscalateServer; @@ -103,6 +104,19 @@ fn approval_sandbox_permissions( } } +fn localize_additional_permissions_for_zsh_fork( + additional_permissions: Option>, +) -> Result, ToolError> { + additional_permissions + .map(localize_additional_permissions_for_current_host) + .transpose() + .map_err(|err| { + ToolError::Rejected(format!( + "zsh-fork cannot localize additional permission URIs: {err}" + )) + }) +} + pub(super) async fn try_run_zsh_fork( req: &ShellRequest, attempt: &SandboxAttempt<'_>, @@ -232,6 +246,8 @@ pub(super) async fn try_run_zsh_fork( req.sandbox_permissions, req.additional_permissions_preapproved, ); + let prompt_permissions = + localize_additional_permissions_for_zsh_fork(req.additional_permissions.clone())?; let escalation_policy = CoreShellActionProvider { policy: Arc::clone(&exec_policy), session: Arc::clone(&ctx.session), @@ -244,7 +260,7 @@ pub(super) async fn try_run_zsh_fork( file_system_sandbox_policy: command_executor.file_system_sandbox_policy.clone(), sandbox_permissions: req.sandbox_permissions, approval_sandbox_permissions, - prompt_permissions: req.additional_permissions.clone(), + prompt_permissions, stopwatch: stopwatch.clone(), }; @@ -316,6 +332,8 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( codex_linux_sandbox_exe: ctx.turn.config.codex_linux_sandbox_exe.clone(), use_legacy_landlock: ctx.turn.config.features.use_legacy_landlock(), }; + let prompt_permissions = + localize_additional_permissions_for_zsh_fork(req.additional_permissions.clone())?; let escalation_policy = CoreShellActionProvider { policy: Arc::clone(&exec_policy), session: Arc::clone(&ctx.session), @@ -331,7 +349,7 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( req.sandbox_permissions, req.additional_permissions_preapproved, ), - prompt_permissions: req.additional_permissions.clone(), + prompt_permissions, stopwatch: Stopwatch::unlimited(), }; @@ -500,8 +518,8 @@ impl CoreShellActionProvider { source, program: program.to_string_lossy().into_owned(), argv: argv.to_vec(), - cwd: workdir.clone(), - additional_permissions, + cwd: PathUri::from_abs_path(&workdir), + additional_permissions: additional_permissions.clone().map(Into::into), }, /*retry_reason*/ None, ) @@ -521,11 +539,11 @@ impl CoreShellActionProvider { approval_id, environment_id, command, - workdir.clone(), + PathUri::from_abs_path(&workdir), /*reason*/ None, /*network_approval_context*/ None, /*proposed_execpolicy_amendment*/ None, - additional_permissions, + additional_permissions.map(Into::into), Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]), ) .await; @@ -1013,7 +1031,7 @@ impl CoreShellCommandExecutor { cwd, env, managed_network: None, - additional_permissions, + additional_permissions: additional_permissions.map(Into::into), }; let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index a1b9b33c6d..886b0e5c42 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -55,7 +55,6 @@ use futures::future::BoxFuture; use std::collections::HashMap; use std::io; use tokio_util::sync::CancellationToken; -use tracing::error; /// Request payload used by the unified-exec runtime after approvals and /// sandbox preferences have been resolved for the current turn. @@ -74,7 +73,7 @@ pub struct UnifiedExecRequest { pub network: Option, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option>, #[cfg(unix)] pub additional_permissions_preapproved: bool, pub justification: Option, @@ -90,7 +89,7 @@ pub struct UnifiedExecApprovalKey { pub cwd: PathUri, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option>, } /// Runtime adapter that keeps policy and sandbox orchestration on the @@ -118,7 +117,7 @@ fn build_unified_exec_sandbox_command( cwd: &PathUri, env: &HashMap, managed_network: Option, - additional_permissions: Option, + additional_permissions: Option>, ) -> Result { let (program, args) = command .split_first() @@ -183,15 +182,6 @@ impl Approvable for UnifiedExecRuntime<'_> { .clone() .or_else(|| req.justification.clone()); Box::pin(async move { - let native_cwd = match req.cwd.to_abs_path() { - Ok(c) => c, - Err(e) => { - // TODO(anp) make sandboxing work for foreign OSes, in the meantime this should - // be impossible for single-OS app-servers - error!(cwd = %req.cwd, ?e, "got non-native path in start_approval_async"); - return ReviewDecision::Abort; - } - }; with_cached_approval(&session.services, "unified_exec", keys, || async move { let available_decisions = None; session @@ -201,7 +191,7 @@ impl Approvable for UnifiedExecRuntime<'_> { /*approval_id*/ None, environment_id, command, - native_cwd, + req.cwd.clone(), reason, ctx.network_approval_context.clone(), req.exec_approval_requirement @@ -224,7 +214,7 @@ impl Approvable for UnifiedExecRuntime<'_> { Ok(ApprovalAction::ExecCommand { id: ctx.call_id.to_string(), command: req.command.clone(), - cwd: req.cwd.to_abs_path()?, + cwd: req.cwd.clone(), sandbox_permissions: req.sandbox_permissions, additional_permissions: req.additional_permissions.clone(), justification: req.justification.clone(), @@ -278,7 +268,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt call_id: ctx.call_id.clone(), tool_name: flat_tool_name(&ctx.tool_name).into_owned(), command: req.command.clone(), - cwd: req.cwd.to_abs_path().ok()?, + cwd: req.cwd.clone(), sandbox_permissions: req.sandbox_permissions, additional_permissions: req.additional_permissions.clone(), justification: req.justification.clone(), diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 571c5ce6df..ccfe9b5557 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -25,7 +25,7 @@ use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; -use codex_sandboxing::policy_transforms::effective_permission_profile; +use codex_sandboxing::policy_transforms::effective_permission_profile_for_uri; use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -480,7 +480,7 @@ impl<'a> SandboxAttempt<'a> { ) -> Result { let network = self.network_proxy(network); let managed_network = command.managed_network.clone(); - let exec_server_permissions = effective_permission_profile( + let exec_server_permissions = effective_permission_profile_for_uri( self.exec_server_permissions, command.additional_permissions.as_ref(), ); @@ -509,7 +509,7 @@ impl<'a> SandboxAttempt<'a> { exec_request.exec_server_managed_network = managed_network; if self.sandbox_requested { exec_request.exec_server_sandbox = Some(FileSystemSandboxContext { - permissions: exec_server_permissions.into(), + permissions: exec_server_permissions, cwd: Some(exec_request.windows_sandbox_policy_cwd.clone()), workspace_roots: Vec::new(), windows_sandbox_level: self.windows_sandbox_level, diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 34d88c8149..7ceeb4ba4d 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -103,7 +103,7 @@ pub(crate) struct ExecCommandRequest { pub network: Option, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option>, pub additional_permissions_preapproved: bool, pub justification: Option, pub prefix_rule: Option>, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 9eb9cb6de4..4a3ea0de20 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -618,7 +618,7 @@ pub enum Op { /// Call id for the in-flight request. id: String, /// User-granted permissions. - response: RequestPermissionsResponse, + response: RequestPermissionsResponse, }, /// Resolve a dynamic tool call request. diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index c9109146bd..4daad82f4b 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -24,6 +24,7 @@ pub use manager::SandboxType; pub use manager::SandboxablePreference; pub use manager::compatibility_sandbox_policy_for_permission_profile; pub use manager::get_platform_sandbox; +pub use manager::localize_additional_permissions_for_current_host; pub use manager::with_managed_mitm_ca_readable_root; pub use windows::WindowsSandboxFilesystemOverrides; pub use windows::permission_profile_supports_windows_restricted_token_sandbox; @@ -45,7 +46,8 @@ impl From for CodexErr { fn from(err: SandboxTransformError) -> Self { match err { error @ SandboxTransformError::InvalidCommandCwd { .. } - | error @ SandboxTransformError::InvalidSandboxPolicyCwd { .. } => { + | error @ SandboxTransformError::InvalidSandboxPolicyCwd { .. } + | error @ SandboxTransformError::InvalidAdditionalPermissions { .. } => { CodexErr::InvalidRequest(error.to_string()) } SandboxTransformError::MissingLinuxSandboxExecutable => { diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 51d2d6c98e..25661f8b96 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -22,6 +22,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::ffi::OsString; @@ -101,7 +102,26 @@ pub struct SandboxCommand { pub cwd: PathUri, pub env: HashMap, pub managed_network: Option, - pub additional_permissions: Option, + pub additional_permissions: Option>, +} + +pub fn localize_additional_permissions_for_current_host( + additional_permissions: AdditionalPermissionProfile, +) -> io::Result { + if let Some(file_system) = additional_permissions.file_system.as_ref() + && let Some((path, _)) = file_system + .explicit_path_entries() + .find(|(path, _)| path.infer_path_convention() != Some(PathConvention::native())) + { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "permission path URI `{path}` is not native to the current host's {} path convention", + PathConvention::native() + ), + )); + } + additional_permissions.try_into() } /// A host-native launch request produced after [`SandboxManager::transform`] validates URI inputs. @@ -211,6 +231,9 @@ pub enum SandboxTransformError { cwd: PathUri, source: io::Error, }, + InvalidAdditionalPermissions { + source: io::Error, + }, MissingLinuxSandboxExecutable, EnvironmentNetworkProxy(String), #[cfg(target_os = "linux")] @@ -234,6 +257,10 @@ impl std::fmt::Display for SandboxTransformError { f, "sandbox policy cwd URI `{cwd}` is not valid on this host: {source}" ), + Self::InvalidAdditionalPermissions { source } => write!( + f, + "additional permission URIs are not valid on this host: {source}" + ), Self::MissingLinuxSandboxExecutable => { write!(f, "missing codex-linux-sandbox executable path") } @@ -256,7 +283,8 @@ impl std::error::Error for SandboxTransformError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { Self::InvalidCommandCwd { source, .. } - | Self::InvalidSandboxPolicyCwd { source, .. } => Some(source), + | Self::InvalidSandboxPolicyCwd { source, .. } + | Self::InvalidAdditionalPermissions { source } => Some(source), Self::MissingLinuxSandboxExecutable => None, Self::EnvironmentNetworkProxy(_) => None, #[cfg(target_os = "linux")] @@ -337,7 +365,18 @@ impl SandboxManager { } = request; #[cfg(target_os = "macos")] let managed_network = command.managed_network.as_ref(); - let additional_permissions = command.additional_permissions.take(); + let additional_permissions = match command + .additional_permissions + .take() + .map(localize_additional_permissions_for_current_host) + .transpose() + { + Ok(additional_permissions) => additional_permissions, + Err(_) if sandbox == SandboxType::None => None, + Err(source) => { + return Err(SandboxTransformError::InvalidAdditionalPermissions { source }); + } + }; let managed_mitm_ca_trust_bundle_path = network.and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); let base_effective_permission_profile =