From 52a50aec709d1934522d7ebdd9ceb4dc75c198f9 Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Fri, 12 Jun 2026 11:38:01 -0700 Subject: [PATCH] sandboxing: migrate cwd inputs to PathUri (#27816) ## Why Sandbox cwd values can cross app-server and exec-server host boundaries. They should retain URI semantics until the receiving host validates them instead of being interpreted early as native paths. ## What - Carry `PathUri` through filesystem sandbox contexts, sandbox commands, and transform inputs. - Convert command and policy cwd once in `SandboxManager::transform`, then keep launch requests native. - Preserve sandbox cwd over remote filesystem transport and reject non-native URIs without fallback. - Cache paired native/URI turn-environment cwd values during migration, with immutable access to keep them synchronized. - Extend existing protocol, forwarding, transform, and core runtime tests. --- codex-rs/Cargo.lock | 2 +- codex-rs/core/src/agents_md.rs | 2 +- codex-rs/core/src/agents_md_tests.rs | 19 ++-- .../core/src/context/environment_context.rs | 2 +- codex-rs/core/src/environment_selection.rs | 36 +++---- codex-rs/core/src/exec.rs | 16 +++- codex-rs/core/src/guardian/review_session.rs | 2 +- codex-rs/core/src/sandboxing/mod.rs | 2 +- codex-rs/core/src/session/mcp.rs | 2 +- codex-rs/core/src/session/session.rs | 2 +- codex-rs/core/src/session/tests.rs | 51 ++++++---- codex-rs/core/src/session/turn.rs | 6 +- codex-rs/core/src/session/turn_context.rs | 41 +++++++- codex-rs/core/src/thread_manager_tests.rs | 16 ++-- .../agent_jobs/spawn_agents_on_csv.rs | 2 +- .../core/src/tools/handlers/apply_patch.rs | 17 +++- .../src/tools/handlers/extension_tools.rs | 25 ++++- .../src/tools/handlers/request_permissions.rs | 2 +- .../handlers/unified_exec/exec_command.rs | 6 +- .../core/src/tools/handlers/view_image.rs | 37 ++++--- codex-rs/core/src/tools/orchestrator.rs | 10 +- .../src/tools/runtimes/apply_patch_tests.rs | 25 +++-- codex-rs/core/src/tools/runtimes/mod.rs | 14 ++- codex-rs/core/src/tools/runtimes/mod_tests.rs | 17 +++- codex-rs/core/src/tools/runtimes/shell.rs | 2 +- .../tools/runtimes/shell/unix_escalation.rs | 17 +++- .../core/src/tools/runtimes/unified_exec.rs | 18 +++- codex-rs/core/src/tools/sandboxing.rs | 28 +++--- codex-rs/core/src/tools/spec_plan_tests.rs | 19 ++-- codex-rs/exec-server/src/fs_sandbox.rs | 96 ++++++++++++++++--- codex-rs/exec-server/src/protocol.rs | 17 +++- .../exec-server/src/remote_file_system.rs | 9 +- .../src/remote_file_system_path_uri_tests.rs | 65 ++++++++++--- .../src/server/file_system_handler.rs | 26 +++-- codex-rs/file-system/Cargo.toml | 1 - codex-rs/file-system/src/lib.rs | 28 +++--- codex-rs/sandboxing/Cargo.toml | 1 + codex-rs/sandboxing/src/lib.rs | 4 + codex-rs/sandboxing/src/manager.rs | 66 +++++++++++-- codex-rs/sandboxing/src/manager_tests.rs | 23 +++-- 40 files changed, 546 insertions(+), 228 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4c22f4a66..10bb76842 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3020,7 +3020,6 @@ name = "codex-file-system" version = "0.0.0" dependencies = [ "codex-protocol", - "codex-utils-absolute-path", "codex-utils-path-uri", "serde", ] @@ -3755,6 +3754,7 @@ dependencies = [ "codex-network-proxy", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "dunce", "libc", "pretty_assertions", diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index c44bcd66e..49ac6fc72 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -57,7 +57,7 @@ pub(crate) async fn load_project_instructions( config, filesystem.as_ref(), &turn_environment.environment_id, - &turn_environment.cwd, + turn_environment.cwd(), ) .await { diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index 2d5bb0d95..e0a046a7e 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -258,14 +258,17 @@ fn resolved_local_environments( ResolvedTurnEnvironments { turn_environments: environments .into_iter() - .map(|(environment_id, cwd)| TurnEnvironment { - environment_id: environment_id.to_string(), - environment: Arc::new( - Environment::create_for_tests(/*exec_server_url*/ None) - .expect("local environment"), - ), - cwd, - shell: None, + .map(|(environment_id, cwd)| { + TurnEnvironment::new( + environment_id.to_string(), + Arc::new( + Environment::create_for_tests(/*exec_server_url*/ None) + .expect("local environment"), + ), + cwd, + /*shell*/ None, + ) + .expect("local cwd URI") }) .collect(), } diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index dd11ebd39..8aa07d1ed 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -46,7 +46,7 @@ impl EnvironmentContextEnvironment { .iter() .map(|environment| Self { id: environment.environment_id.clone(), - cwd: environment.cwd.clone(), + cwd: environment.cwd().clone(), shell: environment .shell .as_ref() diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 0f830f9e0..4e590d3ec 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -58,7 +58,7 @@ impl ResolvedTurnEnvironments { return None; }; - (!environment.environment.is_remote()).then_some(&environment.cwd) + (!environment.environment.is_remote()).then_some(environment.cwd()) } } @@ -96,12 +96,12 @@ pub(crate) async fn resolve_environment_selections( None } }; - turn_environments.push(TurnEnvironment { + turn_environments.push(TurnEnvironment::new( environment_id, environment, - cwd: selected_environment.cwd.clone(), + selected_environment.cwd.clone(), shell, - }); + )?); } Ok(ResolvedTurnEnvironments { turn_environments }) } @@ -270,22 +270,26 @@ url = "ws://127.0.0.1:8765" .expect("remote environment"), ); let remote = ResolvedTurnEnvironments { - turn_environments: vec![TurnEnvironment { - environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - environment: remote_environment.clone(), - cwd: cwd.clone(), - shell: None, - }], + turn_environments: vec![ + TurnEnvironment::new( + REMOTE_ENVIRONMENT_ID.to_string(), + remote_environment.clone(), + cwd.clone(), + /*shell*/ None, + ) + .expect("remote cwd URI"), + ], }; let multiple = ResolvedTurnEnvironments { turn_environments: vec![ local.primary().expect("local environment").clone(), - TurnEnvironment { - environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - environment: remote_environment, - cwd: cwd.clone(), - shell: None, - }, + TurnEnvironment::new( + REMOTE_ENVIRONMENT_ID.to_string(), + remote_environment, + cwd.clone(), + /*shell*/ None, + ) + .expect("remote cwd URI"), ], }; diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 8e87528df..0b3caac36 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -44,6 +44,7 @@ use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use codex_utils_pty::process_group::kill_child_process_group; @@ -372,6 +373,14 @@ pub fn build_exec_request( "command args are empty", )) })?; + let cwd = PathUri::from_abs_path(&cwd).map_err(|_| { + CodexErr::InvalidRequest("command cwd cannot be represented as a file URI".to_string()) + })?; + let sandbox_policy_cwd_uri = PathUri::from_abs_path(sandbox_cwd).map_err(|_| { + CodexErr::InvalidRequest( + "sandbox policy cwd cannot be represented as a file URI".to_string(), + ) + })?; let manager = SandboxManager::new(); let command = SandboxCommand { @@ -392,24 +401,21 @@ pub fn build_exec_request( sandbox: sandbox_type, enforce_managed_network, network: network.as_ref(), - sandbox_policy_cwd: sandbox_cwd, + sandbox_policy_cwd: &sandbox_policy_cwd_uri, codex_linux_sandbox_exe: codex_linux_sandbox_exe.as_deref(), use_legacy_landlock, windows_sandbox_level, windows_sandbox_private_desktop, }) .map(|request| { - let windows_sandbox_policy_cwd = AbsolutePathBuf::try_from(sandbox_cwd.to_path_buf()) - .unwrap_or_else(|_| request.cwd.clone()); let windows_sandbox_workspace_roots = if windows_sandbox_workspace_roots.is_empty() { - vec![windows_sandbox_policy_cwd.clone()] + vec![request.sandbox_policy_cwd.clone()] } else { windows_sandbox_workspace_roots.to_vec() }; ExecRequest::from_sandbox_exec_request( request, options, - windows_sandbox_policy_cwd, windows_sandbox_workspace_roots, ) }) diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 63b778838..f8eb88aaf 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -743,7 +743,7 @@ async fn run_review_on_session( .parent_turn .environments .primary() - .map(|environment| environment.cwd.clone()) + .map(|environment| environment.cwd().clone()) .unwrap_or_else(|| params.parent_turn.config.cwd.clone()); let submit_result = run_before_review_deadline( diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 8a2a2849f..f1f53d80e 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -103,12 +103,12 @@ impl ExecRequest { pub(crate) fn from_sandbox_exec_request( request: SandboxExecRequest, options: ExecOptions, - windows_sandbox_policy_cwd: AbsolutePathBuf, windows_sandbox_workspace_roots: Vec, ) -> Self { let SandboxExecRequest { command, cwd, + sandbox_policy_cwd: windows_sandbox_policy_cwd, mut env, network, sandbox, diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 5e52e6d18..eacbbcee9 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -314,7 +314,7 @@ impl Session { let mcp_runtime_context = match turn_context.environments.primary() { Some(turn_environment) => McpRuntimeContext::new( Arc::clone(&self.services.environment_manager), - turn_environment.cwd.to_path_buf(), + turn_environment.cwd().to_path_buf(), ), None => McpRuntimeContext::new( Arc::clone(&self.services.environment_manager), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index dc118e850..1601a6654 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1135,7 +1135,7 @@ impl Session { let mcp_runtime_context = match turn_environment { Some(turn_environment) => McpRuntimeContext::new( Arc::clone(&sess.services.environment_manager), - turn_environment.cwd.to_path_buf(), + turn_environment.cwd().to_path_buf(), ), None => McpRuntimeContext::new( Arc::clone(&sess.services.environment_manager), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 9ebbdbe0a..8068524d9 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4035,12 +4035,15 @@ fn turn_environments_for_tests( cwd: &codex_utils_absolute_path::AbsolutePathBuf, ) -> crate::environment_selection::ResolvedTurnEnvironments { crate::environment_selection::ResolvedTurnEnvironments { - turn_environments: vec![TurnEnvironment { - environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - environment: Arc::clone(environment), - cwd: cwd.clone(), - shell: None, - }], + turn_environments: vec![ + TurnEnvironment::new( + codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + Arc::clone(environment), + cwd.clone(), + /*shell*/ None, + ) + .expect("turn environment"), + ], } } @@ -5656,8 +5659,14 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir mcp_elicitations: true, })) .expect("test setup should allow updating approval policy"); - turn_context_mut.environments.turn_environments[0].environment_id = "remote".to_string(); - turn_context_mut.environments.turn_environments[0].cwd = environment_cwd.clone(); + let current_environment = turn_context_mut.environments.turn_environments[0].clone(); + turn_context_mut.environments.turn_environments[0] = TurnEnvironment::new( + "remote".to_string(), + current_environment.environment, + environment_cwd.clone(), + current_environment.shell, + ) + .expect("environment cwd URI"); let call_id = "call-1".to_string(); let handler = RequestPermissionsHandler; @@ -6246,15 +6255,15 @@ async fn primary_environment_uses_first_turn_environment() { let first_environment = turn_context.environments.turn_environments[0].clone(); #[allow(deprecated)] let second_cwd = turn_context.cwd.join("second"); - turn_context - .environments - .turn_environments - .push(TurnEnvironment { - environment_id: "second".to_string(), - environment: Arc::clone(&first_environment.environment), - cwd: second_cwd.clone(), - shell: None, - }); + turn_context.environments.turn_environments.push( + TurnEnvironment::new( + "second".to_string(), + Arc::clone(&first_environment.environment), + second_cwd.clone(), + /*shell*/ None, + ) + .expect("turn environment"), + ); assert_eq!( turn_context @@ -6271,13 +6280,13 @@ async fn primary_environment_uses_first_turn_environment() { .iter() .find(|environment| environment.environment_id == "second") .expect("second environment") - .cwd, - second_cwd + .cwd(), + &second_cwd ); assert_eq!(turn_context.environments.turn_environments.len(), 2); assert_eq!( - turn_context.environments.turn_environments[1].cwd, - second_cwd + turn_context.environments.turn_environments[1].cwd(), + &second_cwd ); } diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index da1d82531..e30e93064 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -416,10 +416,10 @@ async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, Pat for turn_environment in &turn_context.environments.turn_environments { let root = get_git_repo_root_with_fs( turn_environment.environment.get_filesystem().as_ref(), - &turn_environment.cwd, + turn_environment.cwd(), ) .await - .unwrap_or_else(|| turn_environment.cwd.clone()) + .unwrap_or_else(|| turn_environment.cwd().clone()) .into_path_buf(); display_roots.push((turn_environment.environment_id.clone(), root)); } @@ -630,7 +630,7 @@ async fn build_extension_turn_input_items( .enumerate() .map(|(index, environment)| TurnInputEnvironment { environment_id: environment.environment_id.clone(), - cwd: environment.cwd.as_path().to_path_buf(), + cwd: environment.cwd().as_path().to_path_buf(), is_primary: index == 0, }) .collect::>(); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index bfadeeb55..99d1482c7 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -17,6 +17,7 @@ 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_utils_path_uri::PathUri; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; @@ -39,11 +40,45 @@ impl TurnSkillsContext { pub(crate) struct TurnEnvironment { pub(crate) environment_id: String, pub(crate) environment: Arc, - pub(crate) cwd: AbsolutePathBuf, + // Keep both representations together while cwd consumers migrate to URI semantics. Keeping + // them synchronized means neither representation can be exposed through a mutable reference; + // updates must rebuild the validated pair through `TurnEnvironment::new`. Once + // `TurnEnvironment::cwd` itself becomes a `PathUri`, convert only at native filesystem and + // process-launch boundaries and remove this paired migration state. + cwd: AbsolutePathBuf, + cwd_uri: PathUri, pub(crate) shell: Option, } impl TurnEnvironment { + pub(crate) fn new( + environment_id: String, + environment: Arc, + cwd: AbsolutePathBuf, + shell: Option, + ) -> CodexResult { + let cwd_uri = PathUri::from_abs_path(&cwd).map_err(|_| { + CodexErr::InvalidRequest( + "turn environment cwd cannot be represented as a file URI".to_string(), + ) + })?; + Ok(Self { + environment_id, + environment, + cwd, + cwd_uri, + shell, + }) + } + + pub(crate) fn cwd(&self) -> &AbsolutePathBuf { + &self.cwd + } + + pub(crate) fn cwd_uri(&self) -> &PathUri { + &self.cwd_uri + } + pub(crate) fn selection(&self) -> TurnEnvironmentSelection { TurnEnvironmentSelection { environment_id: self.environment_id.clone(), @@ -291,7 +326,7 @@ impl TurnContext { pub(crate) fn file_system_sandbox_context( &self, additional_permissions: Option, - cwd: &AbsolutePathBuf, + cwd: &PathUri, ) -> FileSystemSandboxContext { let (base_file_system_sandbox_policy, base_network_sandbox_policy) = self.permission_profile.to_runtime_permissions(); @@ -712,7 +747,7 @@ impl Session { let primary_turn_environment = turn_environments.primary().cloned(); let cwd = primary_turn_environment .as_ref() - .map(|turn_environment| turn_environment.cwd.clone()) + .map(|turn_environment| turn_environment.cwd().clone()) .unwrap_or_else(|| session_configuration.cwd().clone()); let per_turn_config = Self::build_per_turn_config(&session_configuration, cwd.clone()); { diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index fefa5b8ae..790f5ab8c 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -648,12 +648,12 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { .expect("build resumed turn context"); assert_eq!(resumed_turn.environments.turn_environments.len(), 1); assert_eq!( - resumed_turn.environments.turn_environments[0].cwd, - default_cwd + resumed_turn.environments.turn_environments[0].cwd(), + &default_cwd ); assert_ne!( - resumed_turn.environments.turn_environments[0].cwd, - selected_cwd + resumed_turn.environments.turn_environments[0].cwd(), + &selected_cwd ); let forked = manager @@ -675,12 +675,12 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { .expect("build forked turn context"); assert_eq!(forked_turn.environments.turn_environments.len(), 1); assert_eq!( - forked_turn.environments.turn_environments[0].cwd, - default_cwd + forked_turn.environments.turn_environments[0].cwd(), + &default_cwd ); assert_ne!( - forked_turn.environments.turn_environments[0].cwd, - selected_cwd + forked_turn.environments.turn_environments[0].cwd(), + &selected_cwd ); } diff --git a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs index 6da517b9c..255698962 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs @@ -312,5 +312,5 @@ fn single_local_environment_cwd(turn: &TurnContext) -> Result<&AbsolutePathBuf, )); } - Ok(&turn_environment.cwd) + Ok(turn_environment.cwd()) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 70452b665..fc1c43218 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -52,6 +52,7 @@ use codex_sandboxing::policy_transforms::normalize_additional_permissions; use codex_tools::ToolName; use codex_tools::ToolSpec; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; const APPLY_PATCH_ARGUMENT_DIFF_BUFFER_INTERVAL: Duration = Duration::from_millis(500); /// Handles freeform `apply_patch` requests and routes verified patches to the @@ -358,9 +359,12 @@ impl ApplyPatchHandler { "apply_patch is unavailable in this session".to_string(), )); }; - let cwd = turn_environment.cwd.clone(); + let cwd = turn_environment.cwd().clone(); let fs = turn_environment.environment.get_filesystem(); - let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, &cwd); + let sandbox = turn.file_system_sandbox_context( + /*additional_permissions*/ None, + turn_environment.cwd_uri(), + ); match codex_apply_patch::verify_apply_patch_args(args, &cwd, fs.as_ref(), Some(&sandbox)) .await { @@ -522,7 +526,14 @@ 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_cwd = PathUri::from_abs_path(cwd).map_err(|_| { + FunctionCallError::RespondToModel( + "unable to prepare filesystem sandbox: working directory cannot be represented as a file URI" + .to_string(), + ) + })?; + let sandbox = + turn.file_system_sandbox_context(/*additional_permissions*/ None, &sandbox_cwd); match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, Some(&sandbox)) .await { diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index 2da47fc7b..33625317f 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -117,19 +117,20 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { let additional_permissions = apply_granted_turn_permissions( invocation.session.as_ref(), &environment.environment_id, - environment.cwd.as_path(), + environment.cwd().as_path(), SandboxPermissions::UseDefault, /*additional_permissions*/ None, ) .await .additional_permissions; + let file_system_sandbox_context = invocation + .turn + .file_system_sandbox_context(additional_permissions, environment.cwd_uri()); environments.push(ToolEnvironment { environment_id: environment.environment_id.clone(), - cwd: environment.cwd.clone(), + cwd: environment.cwd().clone(), file_system: environment.environment.get_filesystem(), - file_system_sandbox_context: invocation - .turn - .file_system_sandbox_context(additional_permissions, &environment.cwd), + file_system_sandbox_context, }); } ExtensionToolCall { @@ -310,6 +311,12 @@ mod tests { let turn_id = turn.sub_id.clone(); let model = turn.model_info.slug.clone(); let truncation_policy = turn.truncation_policy; + let expected_sandbox_cwds = turn + .environments + .turn_environments + .iter() + .map(|environment| Some(environment.cwd_uri().clone())) + .collect::>(); let history_item = ResponseItem::Message { id: None, role: "user".to_string(), @@ -354,6 +361,14 @@ mod tests { ); assert_eq!(captured_call.model, model); assert_eq!(captured_call.truncation_policy, truncation_policy); + assert_eq!( + captured_call + .environments + .iter() + .map(|environment| environment.file_system_sandbox_context.cwd.clone()) + .collect::>(), + expected_sandbox_cwds + ); assert_eq!( captured_call.conversation_history.items(), std::slice::from_ref(&history_item) diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 4faf9d328..53341b14f 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -71,7 +71,7 @@ impl RequestPermissionsHandler { )); }; let mut args: RequestPermissionsArgs = - parse_arguments_with_base_path(&arguments, &turn_environment.cwd)?; + parse_arguments_with_base_path(&arguments, turn_environment.cwd())?; args.permissions = normalize_additional_permissions(args.permissions.into()) .map(codex_protocol::request_permissions::RequestPermissionProfile::from) .map_err(FunctionCallError::RespondToModel)?; 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 7c914deb4..55d3befee 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 @@ -134,8 +134,8 @@ impl ExecCommandHandler { .as_deref() .filter(|workdir| !workdir.is_empty()) .map_or_else( - || turn_environment.cwd.clone(), - |workdir| turn_environment.cwd.join(workdir), + || turn_environment.cwd().clone(), + |workdir| turn_environment.cwd().join(workdir), ); let environment = Arc::clone(&turn_environment.environment); let fs = environment.get_filesystem(); @@ -270,7 +270,7 @@ impl ExecCommandHandler { yield_time_ms, max_output_tokens, cwd, - sandbox_cwd: turn_environment.cwd.clone(), + sandbox_cwd: turn_environment.cwd().clone(), environment, shell_mode, network: context.turn.network.clone(), diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 17708564b..6a66a43a8 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -143,9 +143,12 @@ impl ViewImageHandler { "view_image is unavailable in this session".to_string(), )); }; - let cwd = turn_environment.cwd.clone(); + let cwd = turn_environment.cwd().clone(); let abs_path = cwd.join(path); - let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, &cwd); + let sandbox = turn.file_system_sandbox_context( + /*additional_permissions*/ None, + turn_environment.cwd_uri(), + ); let fs = turn_environment.environment.get_filesystem(); let path_uri = PathUri::from_abs_path(&abs_path).map_err(|error| { FunctionCallError::RespondToModel(format!( @@ -268,16 +271,34 @@ impl ToolOutput for ViewImageOutput { mod tests { use super::*; use crate::session::tests::make_session_and_context; + use crate::session::turn_context::TurnEnvironment; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::turn_diff_tracker::TurnDiffTracker; use codex_protocol::models::PermissionProfile; + use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::TempDirExt; use pretty_assertions::assert_eq; use serde_json::json; use std::sync::Arc; use tokio::sync::Mutex; + fn replace_primary_environment_cwd(turn: &mut crate::TurnContext, cwd: AbsolutePathBuf) { + let current = turn + .environments + .turn_environments + .first() + .cloned() + .expect("default local turn environment"); + turn.environments.turn_environments[0] = TurnEnvironment::new( + current.environment_id, + current.environment, + cwd, + current.shell, + ) + .expect("image cwd URI"); + } + #[test] fn log_preview_omits_image_data() { let output = ViewImageOutput { @@ -314,11 +335,7 @@ mod tests { let image_dir = tempfile::tempdir().expect("create image temp dir"); let image_cwd = image_dir.abs(); - turn.environments - .turn_environments - .first_mut() - .expect("default local turn environment") - .cwd = image_cwd.clone(); + replace_primary_environment_cwd(&mut turn, image_cwd.clone()); let image_path = image_cwd.join("image.png"); std::fs::write(image_path.as_path(), b"not a real image").expect("write test image"); turn.permission_profile = PermissionProfile::read_only(); @@ -381,11 +398,7 @@ mod tests { let image_dir = tempfile::tempdir().expect("create image temp dir"); let image_cwd = image_dir.abs(); - turn.environments - .turn_environments - .first_mut() - .expect("default local turn environment") - .cwd = image_cwd.clone(); + replace_primary_environment_cwd(&mut turn, image_cwd.clone()); let image_path = image_cwd.join("image.png"); std::fs::write(image_path.as_path(), b"not a real image").expect("write test image"); turn.permission_profile = PermissionProfile::Disabled; diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 77cbffde3..f0b0f8ca0 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -39,6 +39,7 @@ use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxType; +use codex_utils_path_uri::PathUri; use std::time::Instant; pub(crate) struct ToolOrchestrator { @@ -239,13 +240,18 @@ impl ToolOrchestrator { let use_legacy_landlock = turn_ctx.features.use_legacy_landlock(); #[allow(deprecated)] let sandbox_cwd = tool.sandbox_cwd(req).unwrap_or(&turn_ctx.cwd); + let sandbox_policy_cwd = PathUri::from_abs_path(sandbox_cwd).map_err(|_| { + ToolError::Codex(CodexErr::InvalidRequest( + "sandbox policy cwd cannot be represented as a file URI".to_string(), + )) + })?; let workspace_roots = turn_ctx.config.effective_workspace_roots(); let initial_attempt = SandboxAttempt { sandbox: initial_sandbox, permissions: &turn_ctx.permission_profile, enforce_managed_network: managed_network_active, manager: &self.sandbox, - sandbox_cwd, + sandbox_cwd: &sandbox_policy_cwd, workspace_roots: workspace_roots.as_slice(), codex_linux_sandbox_exe: turn_ctx.codex_linux_sandbox_exe.as_ref(), use_legacy_landlock, @@ -418,7 +424,7 @@ impl ToolOrchestrator { permissions: &turn_ctx.permission_profile, enforce_managed_network: managed_network_active, manager: &self.sandbox, - sandbox_cwd, + sandbox_cwd: &sandbox_policy_cwd, workspace_roots: workspace_roots.as_slice(), codex_linux_sandbox_exe: retry_codex_linux_sandbox_exe, use_legacy_landlock, 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 0a741861f..e3b1cc90a 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -11,16 +11,18 @@ use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxType; use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; use codex_sandboxing::policy_transforms::effective_network_sandbox_policy; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use pretty_assertions::assert_eq; use std::collections::HashMap; fn test_turn_environment(environment_id: &str) -> crate::session::turn_context::TurnEnvironment { - crate::session::turn_context::TurnEnvironment { - environment_id: environment_id.to_string(), - environment: std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()), - cwd: std::env::temp_dir().abs(), - shell: None, - } + crate::session::turn_context::TurnEnvironment::new( + environment_id.to_string(), + std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()), + std::env::temp_dir().abs(), + /*shell*/ None, + ) + .expect("turn environment") } #[test] @@ -206,12 +208,13 @@ async fn file_system_sandbox_context_uses_active_attempt() { NetworkSandboxPolicy::Restricted, ); let manager = SandboxManager::new(); + let sandbox_policy_cwd = PathUri::from_abs_path(&path).expect("path URI"); let attempt = SandboxAttempt { sandbox: SandboxType::MacosSeatbelt, permissions: &permissions, enforce_managed_network: false, manager: &manager, - sandbox_cwd: &path, + sandbox_cwd: &sandbox_policy_cwd, workspace_roots: std::slice::from_ref(&path), codex_linux_sandbox_exe: None, use_legacy_landlock: true, @@ -232,7 +235,10 @@ async fn file_system_sandbox_context_uses_active_attempt() { let expected_permissions = PermissionProfile::from_runtime_permissions(&file_system_policy, network_policy); assert_eq!(sandbox.permissions, expected_permissions); - assert_eq!(sandbox.cwd, Some(path.clone())); + assert_eq!( + sandbox.cwd, + Some(codex_utils_path_uri::PathUri::from_abs_path(&path).expect("path URI")) + ); assert_eq!( sandbox.windows_sandbox_level, WindowsSandboxLevel::RestrictedToken @@ -260,12 +266,13 @@ async fn no_sandbox_attempt_has_no_file_system_context() { }; let permissions = PermissionProfile::Disabled; let manager = SandboxManager::new(); + let sandbox_policy_cwd = PathUri::from_abs_path(&path).expect("path URI"); let attempt = SandboxAttempt { sandbox: SandboxType::None, permissions: &permissions, enforce_managed_network: false, manager: &manager, - sandbox_cwd: &path, + sandbox_cwd: &sandbox_policy_cwd, workspace_roots: std::slice::from_ref(&path), codex_linux_sandbox_exe: None, use_legacy_landlock: false, diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 29d323b63..241ce4a54 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -21,10 +21,12 @@ use codex_network_proxy::PROXY_ENV_KEYS; use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY; use codex_network_proxy::is_managed_mitm_ca_trust_bundle_path; use codex_protocol::config_types::WindowsSandboxLevel; +use codex_protocol::error::CodexErr; use codex_protocol::models::AdditionalPermissionProfile; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::collections::HashMap; #[cfg(unix)] use std::path::Path; @@ -33,8 +35,9 @@ pub(crate) mod apply_patch; pub(crate) mod shell; pub(crate) mod unified_exec; -/// Shared helper to construct sandbox transform inputs from a tokenized command line. -/// Validates that at least a program is present. +/// Shared helper to construct sandbox transform inputs from a tokenized command line and native +/// working directory. Validates that at least a program is present and that the working directory +/// has a file URI representation. pub(crate) fn build_sandbox_command( command: &[String], cwd: &AbsolutePathBuf, @@ -44,10 +47,15 @@ pub(crate) fn build_sandbox_command( let (program, args) = command .split_first() .ok_or_else(|| ToolError::Rejected("command args are empty".to_string()))?; + let cwd = PathUri::from_abs_path(cwd).map_err(|_| { + ToolError::Codex(CodexErr::InvalidRequest( + "command cwd cannot be represented as a file URI".to_string(), + )) + })?; Ok(SandboxCommand { program: program.clone().into(), args: args.to_vec(), - cwd: cwd.clone(), + cwd, env: env.clone(), additional_permissions, }) diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index 99cece3e9..33f29991c 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -25,6 +25,7 @@ use codex_protocol::models::PermissionProfile; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; use pretty_assertions::assert_eq; @@ -88,18 +89,21 @@ async fn test_network_proxy() -> anyhow::Result { async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow::Result<()> { let proxy = test_network_proxy().await?; let dir = tempdir().expect("create temp dir"); - let cwd = dir.path().abs(); + let command_cwd = dir.path().join("command").abs(); + let native_sandbox_policy_cwd = dir.path().join("sandbox-policy").abs(); let mut env = HashMap::from([("CUSTOM_ENV".to_string(), "kept".to_string())]); proxy.apply_to_env(&mut env); let command = vec!["/bin/echo".to_string(), "ok".to_string()]; let command = build_sandbox_command( &command, - &cwd, + &command_cwd, &exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated), /*additional_permissions*/ None, ) .expect("build sandbox command"); + assert_eq!(command.cwd, PathUri::from_abs_path(&command_cwd)?); + let sandbox_policy_cwd = PathUri::from_abs_path(&native_sandbox_policy_cwd)?; let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, capture_policy: ExecCapturePolicy::ShellTool, @@ -111,8 +115,8 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: permissions: &permissions, enforce_managed_network: false, manager: &manager, - sandbox_cwd: &cwd, - workspace_roots: std::slice::from_ref(&cwd), + sandbox_cwd: &sandbox_policy_cwd, + workspace_roots: std::slice::from_ref(&native_sandbox_policy_cwd), codex_linux_sandbox_exe: None, use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, @@ -131,6 +135,11 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: ) .expect("prepare exec request"); + assert_eq!(exec_request.cwd, command_cwd); + assert_eq!( + exec_request.windows_sandbox_policy_cwd, + native_sandbox_policy_cwd + ); assert_eq!(exec_request.network, None); for key in PROXY_ENV_KEYS { assert_eq!(exec_request.env.get(*key), None, "{key} should be unset"); diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 13b65f044..612ab8c0f 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -310,7 +310,7 @@ impl ToolRuntime for ShellRuntime { }; let env = attempt .env_for(command, options, managed_network) - .map_err(|err| ToolError::Codex(err.into()))?; + .map_err(ToolError::Codex)?; let out = execute_env(env, Self::stdout_stream(ctx)) .await .map_err(ToolError::Codex)?; 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 960e1d394..7e854d8ef 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -66,6 +66,7 @@ use codex_shell_escalation::ShellCommandExecutor; use codex_shell_escalation::ShellCommandExecutorFuture; use codex_shell_escalation::Stopwatch; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; @@ -143,7 +144,7 @@ pub(super) async fn try_run_zsh_fork( options, managed_network_for_sandbox_permissions(req.network.as_ref(), req.sandbox_permissions), ) - .map_err(|err| ToolError::Codex(err.into()))?; + .map_err(ToolError::Codex)?; let crate::sandboxing::ExecRequest { command, cwd: sandbox_cwd, @@ -968,10 +969,19 @@ impl CoreShellCommandExecutor { self.windows_sandbox_level, self.network.is_some(), ); + let cwd = PathUri::from_abs_path(workdir).map_err(|_| { + CodexErr::InvalidRequest("command cwd cannot be represented as a file URI".to_string()) + })?; + let sandbox_policy_cwd = + PathUri::from_abs_path(&self.sandbox_policy_cwd).map_err(|_| { + CodexErr::InvalidRequest( + "sandbox policy cwd cannot be represented as a file URI".to_string(), + ) + })?; let command = SandboxCommand { program: program.clone().into(), args: args.to_vec(), - cwd: workdir.clone(), + cwd, env, additional_permissions, }; @@ -985,7 +995,7 @@ impl CoreShellCommandExecutor { sandbox, enforce_managed_network: self.network.is_some(), network: self.network.as_ref(), - sandbox_policy_cwd: &self.sandbox_policy_cwd, + sandbox_policy_cwd: &sandbox_policy_cwd, codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.as_deref(), use_legacy_landlock: self.use_legacy_landlock, windows_sandbox_level: self.windows_sandbox_level, @@ -994,7 +1004,6 @@ impl CoreShellCommandExecutor { let mut exec_request = crate::sandboxing::ExecRequest::from_sandbox_exec_request( exec_request, options, - self.sandbox_policy_cwd.clone(), self.windows_sandbox_workspace_roots.clone(), ); if let Some(network) = exec_request.network.as_ref() { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 250965b67..9aff1ac79 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -326,11 +326,16 @@ impl<'a> ToolRuntime for UnifiedExecRunt if let UnifiedExecShellMode::ZshFork(zsh_fork_config) = &self.shell_mode { let command = build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone()) - .map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?; + .map_err(|error| match error { + ToolError::Rejected(_) => { + ToolError::Rejected("missing command line for PTY".to_string()) + } + error @ ToolError::Codex(_) => error, + })?; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); let mut exec_env = attempt .env_for(command, options, managed_network) - .map_err(|err| ToolError::Codex(err.into()))?; + .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); match zsh_fork_backend::maybe_prepare_unified_exec( req, @@ -377,11 +382,16 @@ impl<'a> ToolRuntime for UnifiedExecRunt } let command = build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone()) - .map_err(|_| ToolError::Rejected("missing command line for PTY".to_string()))?; + .map_err(|error| match error { + ToolError::Rejected(_) => { + ToolError::Rejected("missing command line for PTY".to_string()) + } + error @ ToolError::Codex(_) => error, + })?; let options = unified_exec_options(attempt.network_denial_cancellation_token.clone()); let mut exec_env = attempt .env_for(command, options, managed_network) - .map_err(|err| ToolError::Codex(err.into()))?; + .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); self.manager .open_session_with_exec_env( diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 598d8855a..499cdfbda 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -21,12 +21,12 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxManager; -use codex_sandboxing::SandboxTransformError; use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxType; use codex_sandboxing::SandboxablePreference; use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::Future; use futures::future::BoxFuture; use serde::Serialize; @@ -411,7 +411,7 @@ pub(crate) struct SandboxAttempt<'a> { pub permissions: &'a codex_protocol::models::PermissionProfile, pub enforce_managed_network: bool, pub(crate) manager: &'a SandboxManager, - pub(crate) sandbox_cwd: &'a AbsolutePathBuf, + pub(crate) sandbox_cwd: &'a PathUri, pub(crate) workspace_roots: &'a [AbsolutePathBuf], pub codex_linux_sandbox_exe: Option<&'a std::path::PathBuf>, pub use_legacy_landlock: bool, @@ -426,8 +426,9 @@ impl<'a> SandboxAttempt<'a> { command: SandboxCommand, options: ExecOptions, network: Option<&NetworkProxy>, - ) -> Result { - self.manager + ) -> Result { + let request = self + .manager .transform(SandboxTransformRequest { command, permissions: self.permissions, @@ -442,19 +443,12 @@ impl<'a> SandboxAttempt<'a> { windows_sandbox_level: self.windows_sandbox_level, windows_sandbox_private_desktop: self.windows_sandbox_private_desktop, }) - .map(|request| { - let windows_sandbox_policy_cwd = - codex_utils_absolute_path::AbsolutePathBuf::try_from( - self.sandbox_cwd.to_path_buf(), - ) - .unwrap_or_else(|_| request.cwd.clone()); - crate::sandboxing::ExecRequest::from_sandbox_exec_request( - request, - options, - windows_sandbox_policy_cwd, - self.workspace_roots.to_vec(), - ) - }) + .map_err(CodexErr::from)?; + Ok(crate::sandboxing::ExecRequest::from_sandbox_exec_request( + request, + options, + self.workspace_roots.to_vec(), + )) } } diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 3f9832c85..8e28c1c36 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -573,21 +573,22 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava .environments .primary() .expect("primary environment") - .cwd + .cwd() .clone(); - turn.environments - .turn_environments - .push(crate::session::turn_context::TurnEnvironment { - environment_id: "remote".to_string(), - environment: Arc::new( + turn.environments.turn_environments.push( + crate::session::turn_context::TurnEnvironment::new( + "remote".to_string(), + Arc::new( codex_exec_server::Environment::create_for_tests(Some( "ws://127.0.0.1:1/remote-exec-server".to_string(), )) .expect("remote test environment"), ), - cwd: remote_cwd, - shell: None, - }); + remote_cwd, + /*shell*/ None, + ) + .expect("turn environment"), + ); }) .await; diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 8ef20b2d1..9c88c6b88 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -15,6 +15,7 @@ use codex_sandboxing::SandboxTransformRequest; use codex_sandboxing::SandboxablePreference; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; +use codex_utils_path_uri::PathUri; use tokio::io::AsyncWriteExt; use tokio::process::Command; @@ -39,6 +40,12 @@ const FS_HELPER_BAZEL_BWRAP_ENV_ALLOWLIST: &[&str] = &[ "TEST_WORKSPACE", ]; +#[derive(Debug, PartialEq, Eq)] +struct SandboxCwd { + uri: PathUri, + native: AbsolutePathBuf, +} + #[derive(Clone, Debug)] pub(crate) struct FileSystemSandboxRunner { runtime_paths: ExecServerRuntimePaths, @@ -65,7 +72,11 @@ impl FileSystemSandboxRunner { } else { helper_read_roots(&self.runtime_paths) }; - add_helper_runtime_permissions(&mut file_system_policy, &helper_read_roots, cwd.as_path()); + add_helper_runtime_permissions( + &mut file_system_policy, + &helper_read_roots, + cwd.native.as_path(), + ); normalize_file_system_policy_root_aliases(&mut file_system_policy); let network_policy = NetworkSandboxPolicy::Restricted; let permission_profile = PermissionProfile::from_runtime_permissions_with_enforcement( @@ -73,7 +84,7 @@ impl FileSystemSandboxRunner { &file_system_policy, network_policy, ); - let command = self.sandbox_exec_request(&permission_profile, &cwd, sandbox)?; + let command = self.sandbox_exec_request(&permission_profile, &cwd.uri, sandbox)?; let request_json = serde_json::to_vec(&request).map_err(json_error)?; run_command(command, request_json).await } @@ -81,7 +92,7 @@ impl FileSystemSandboxRunner { fn sandbox_exec_request( &self, permission_profile: &PermissionProfile, - cwd: &AbsolutePathBuf, + cwd: &PathUri, sandbox_context: &FileSystemSandboxContext, ) -> Result { let helper = &self.runtime_paths.codex_self_exe; @@ -108,7 +119,7 @@ impl FileSystemSandboxRunner { sandbox, enforce_managed_network: false, network: None, - sandbox_policy_cwd: cwd.as_path(), + sandbox_policy_cwd: cwd, codex_linux_sandbox_exe: self.runtime_paths.codex_linux_sandbox_exe.as_deref(), use_legacy_landlock: sandbox_context.use_legacy_landlock, windows_sandbox_level: sandbox_context.windows_sandbox_level, @@ -118,9 +129,12 @@ impl FileSystemSandboxRunner { } } -fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result { - if let Some(cwd) = &sandbox.cwd { - return Ok(cwd.clone()); +fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result { + if let Some(uri) = &sandbox.cwd { + return Ok(SandboxCwd { + native: native_sandbox_cwd(uri)?, + uri: uri.clone(), + }); } if sandbox.has_cwd_dependent_permissions() { @@ -129,9 +143,22 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result Result { + cwd.to_abs_path().map_err(|err| { + invalid_request(format!( + "file system sandbox cwd is not native to this exec-server host: {err}" + )) + }) } fn helper_read_roots(runtime_paths: &ExecServerRuntimePaths) -> Vec { @@ -327,11 +354,13 @@ mod tests { use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use crate::ExecServerRuntimePaths; use super::FileSystemSandboxRunner; + use super::SandboxCwd; use super::add_helper_runtime_permissions; use super::helper_env; use super::helper_env_from_vars; @@ -489,9 +518,10 @@ mod tests { ExecServerRuntimePaths::new(codex_self_exe.clone(), Some(codex_self_exe)) .expect("runtime paths"); let runner = FileSystemSandboxRunner::new(runtime_paths); - let cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let native_cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let cwd = PathUri::from_abs_path(&native_cwd).expect("cwd URI"); let file_system_policy = - restricted_policy(vec![path_entry(cwd.clone(), FileSystemAccessMode::Write)]); + restricted_policy(vec![path_entry(native_cwd, FileSystemAccessMode::Write)]); let network_policy = NetworkSandboxPolicy::Restricted; let permission_profile = PermissionProfile::from_runtime_permissions(&file_system_policy, network_policy); @@ -506,15 +536,42 @@ mod tests { #[test] fn sandbox_cwd_uses_context_cwd() { - let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) + let native_cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path()) .expect("absolute cwd"); + let cwd = PathUri::from_abs_path(&native_cwd).expect("cwd URI"); let policy = restricted_policy(vec![special_entry( FileSystemSpecialPath::project_roots(/*subpath*/ None), FileSystemAccessMode::Write, )]); let sandbox_context = sandbox_context_with_cwd(&policy, cwd.clone()); - assert_eq!(sandbox_cwd(&sandbox_context).expect("sandbox cwd"), cwd); + assert_eq!( + sandbox_cwd(&sandbox_context).expect("sandbox cwd"), + SandboxCwd { + uri: cwd, + native: native_cwd + } + ); + } + + #[test] + fn sandbox_cwd_rejects_non_native_context_cwd_without_fallback() { + let cwd = non_native_cwd(); + let policy = restricted_policy(vec![special_entry( + FileSystemSpecialPath::project_roots(/*subpath*/ None), + FileSystemAccessMode::Write, + )]); + let sandbox_context = sandbox_context_with_cwd(&policy, cwd); + + let err = sandbox_cwd(&sandbox_context).expect_err("non-native cwd should be rejected"); + + assert_eq!( + err, + crate::rpc::invalid_request( + "file system sandbox cwd is not native to this exec-server host: file URI contains an invalid absolute path" + .to_string() + ) + ); } #[test] @@ -595,7 +652,7 @@ mod tests { fn sandbox_context_with_cwd( policy: &FileSystemSandboxPolicy, - cwd: AbsolutePathBuf, + cwd: PathUri, ) -> crate::FileSystemSandboxContext { crate::FileSystemSandboxContext::from_permission_profile_with_cwd( PermissionProfile::from_runtime_permissions(policy, NetworkSandboxPolicy::Restricted), @@ -603,6 +660,15 @@ mod tests { ) } + fn non_native_cwd() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/checkout"; + #[cfg(windows)] + let uri = "file:///usr/local/checkout"; + + PathUri::parse(uri).expect("non-native cwd URI") + } + fn path_entry(path: AbsolutePathBuf, access: FileSystemAccessMode) -> FileSystemSandboxEntry { FileSystemSandboxEntry { path: FileSystemPath::Path { path }, diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 8b9819b64..387f48b35 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -450,6 +450,8 @@ mod base64_bytes { mod tests { use super::FsReadFileParams; use super::HttpRequestParams; + use crate::FileSystemSandboxContext; + use codex_protocol::models::PermissionProfile; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; @@ -458,14 +460,22 @@ mod tests { let legacy_path = std::env::current_dir() .expect("current directory") .join("legacy-file.txt"); + let legacy_cwd = std::env::current_dir().expect("current directory"); + let expected_sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd( + PermissionProfile::default(), + PathUri::from_path(&legacy_cwd).expect("cwd URI"), + ); + let mut legacy_sandbox = + serde_json::to_value(&expected_sandbox).expect("sandbox should serialize"); + legacy_sandbox["cwd"] = serde_json::json!(legacy_cwd.to_string_lossy()); let params: FsReadFileParams = serde_json::from_value(serde_json::json!({ "path": legacy_path.to_string_lossy(), - "sandbox": null, + "sandbox": legacy_sandbox, })) .expect("legacy absolute path should deserialize"); let expected = FsReadFileParams { path: PathUri::from_path(legacy_path).expect("path URI"), - sandbox: None, + sandbox: Some(expected_sandbox.clone()), }; assert_eq!(params, expected); @@ -473,7 +483,8 @@ mod tests { serde_json::to_value(params).expect("params should serialize"), serde_json::json!({ "path": expected.path.to_string(), - "sandbox": null, + "sandbox": serde_json::to_value(expected_sandbox) + .expect("sandbox should serialize"), }) ); } diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 03fe0532e..f6924d780 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -321,6 +321,7 @@ mod tests { use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use super::*; @@ -337,7 +338,7 @@ mod tests { PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted); let sandbox_context = FileSystemSandboxContext::from_permission_profile_with_cwd( permissions, - absolute_test_path("host-checkout"), + path_uri("host-checkout"), ); let remote_context = @@ -356,7 +357,7 @@ mod tests { }]); let permissions = PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted); - let cwd = absolute_test_path("host-checkout"); + let cwd = path_uri("host-checkout"); let sandbox_context = FileSystemSandboxContext::from_permission_profile_with_cwd(permissions, cwd.clone()); @@ -400,4 +401,8 @@ mod tests { let path = std::env::temp_dir().join(name); AbsolutePathBuf::from_absolute_path(&path).expect("absolute path") } + + fn path_uri(name: &str) -> PathUri { + PathUri::from_abs_path(&absolute_test_path(name)).expect("path URI") + } } diff --git a/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs index 47703f870..29284e6ad 100644 --- a/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs +++ b/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs @@ -2,6 +2,13 @@ use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::JSONRPCResponse; +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_utils_path_uri::PathUri; use futures::SinkExt; use futures::StreamExt; @@ -25,9 +32,9 @@ use crate::protocol::INITIALIZED_METHOD; use crate::protocol::InitializeResponse; #[tokio::test] -async fn remote_file_system_sends_path_uris_without_native_conversion() { - let (websocket_url, captured_paths, server) = - record_read_file_paths(/*expected_requests*/ 2).await; +async fn remote_file_system_sends_path_and_sandbox_cwd_uris_without_native_conversion() { + let (websocket_url, captured_params, server) = + record_read_file_params(/*expected_requests*/ 2).await; let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new( ExecServerTransportParams::websocket_url(websocket_url), )); @@ -35,33 +42,54 @@ async fn remote_file_system_sends_path_uris_without_native_conversion() { PathUri::parse("file:///C:/Users/Alice/src/main.rs").expect("valid drive URI"), PathUri::parse("file://server/share/src/main.rs").expect("valid UNC URI"), ]; + let sandbox_cwd = non_native_cwd(); + let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { + path: FileSystemPath::Special { + value: FileSystemSpecialPath::project_roots(/*subpath*/ None), + }, + access: FileSystemAccessMode::Write, + }]); + let sandbox = FileSystemSandboxContext::from_permission_profile_with_cwd( + PermissionProfile::from_runtime_permissions(&policy, NetworkSandboxPolicy::Restricted), + sandbox_cwd, + ); for path in &paths { assert_eq!( file_system - .read_file(path, /*sandbox*/ None) + .read_file(path, Some(&sandbox)) .await .expect("remote read should succeed"), Vec::::new() ); } - assert_eq!(captured_paths.await.expect("captured paths"), paths); + let expected_params = paths + .into_iter() + .map(|path| FsReadFileParams { + path, + sandbox: Some(sandbox.clone()), + }) + .collect::>(); + assert_eq!( + captured_params.await.expect("captured params"), + expected_params + ); server.await.expect("recording server should succeed"); } -async fn record_read_file_paths( +async fn record_read_file_params( expected_requests: usize, ) -> ( String, - oneshot::Receiver>, + oneshot::Receiver>, tokio::task::JoinHandle<()>, ) { let listener = TcpListener::bind("127.0.0.1:0") .await .expect("listener should bind"); let websocket_url = format!("ws://{}", listener.local_addr().expect("listener address")); - let (captured_paths_tx, captured_paths_rx) = oneshot::channel(); + let (captured_params_tx, captured_params_rx) = oneshot::channel(); let server = tokio::spawn(async move { let (stream, _) = listener.accept().await.expect("listener should accept"); let mut websocket = accept_async(stream) @@ -69,7 +97,7 @@ async fn record_read_file_paths( .expect("websocket handshake should succeed"); complete_websocket_initialize(&mut websocket).await; - let mut captured_paths = Vec::with_capacity(expected_requests); + let mut captured_params = Vec::with_capacity(expected_requests); for _ in 0..expected_requests { let request = match read_jsonrpc_websocket(&mut websocket).await { JSONRPCMessage::Request(request) if request.method == FS_READ_FILE_METHOD => { @@ -80,7 +108,7 @@ async fn record_read_file_paths( let params: FsReadFileParams = serde_json::from_value(request.params.expect("fs/readFile params should exist")) .expect("fs/readFile params should deserialize"); - captured_paths.push(params.path); + captured_params.push(params); write_jsonrpc_websocket( &mut websocket, JSONRPCMessage::Response(JSONRPCResponse { @@ -93,12 +121,21 @@ async fn record_read_file_paths( ) .await; } - captured_paths_tx - .send(captured_paths) - .expect("captured paths receiver should stay open"); + captured_params_tx + .send(captured_params) + .expect("captured params receiver should stay open"); }); - (websocket_url, captured_paths_rx, server) + (websocket_url, captured_params_rx, server) +} + +fn non_native_cwd() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/checkout"; + #[cfg(windows)] + let uri = "file:///usr/local/checkout"; + + PathUri::parse(uri).expect("non-native cwd URI") } async fn complete_websocket_initialize(websocket: &mut WebSocketStream) { diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index 1b6281f25..211685c13 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -189,7 +189,6 @@ fn map_fs_error(err: io::Error) -> JSONRPCErrorError { mod tests { use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::SandboxPolicy; - use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; @@ -207,8 +206,14 @@ mod tests { ) .expect("runtime paths"); let handler = FileSystemHandler::new(runtime_paths); - let sandbox_cwd = - AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute tempdir"); + let sandbox_cwd = PathUri::from_path(temp_dir.path()).expect("tempdir URI"); + let sandbox_context = |sandbox_policy| { + FileSystemSandboxContext::from_legacy_sandbox_policy( + sandbox_policy, + sandbox_cwd.clone(), + ) + .expect("sandbox context") + }; for (file_name, sandbox_policy) in [ ("danger.txt", SandboxPolicy::DangerFullAccess), @@ -225,10 +230,7 @@ mod tests { .write_file(FsWriteFileParams { path: path.clone(), data_base64: STANDARD.encode("ok"), - sandbox: Some(FileSystemSandboxContext::from_legacy_sandbox_policy( - sandbox_policy.clone(), - sandbox_cwd.clone(), - )), + sandbox: Some(sandbox_context(sandbox_policy.clone())), }) .await .expect("write file"); @@ -236,10 +238,7 @@ mod tests { let canonicalized = handler .canonicalize(FsCanonicalizeParams { path: path.clone(), - sandbox: Some(FileSystemSandboxContext::from_legacy_sandbox_policy( - sandbox_policy.clone(), - sandbox_cwd.clone(), - )), + sandbox: Some(sandbox_context(sandbox_policy.clone())), }) .await .expect("canonicalize file"); @@ -254,10 +253,7 @@ mod tests { let response = handler .read_file(FsReadFileParams { path, - sandbox: Some(FileSystemSandboxContext::from_legacy_sandbox_policy( - sandbox_policy, - sandbox_cwd.clone(), - )), + sandbox: Some(sandbox_context(sandbox_policy)), }) .await .expect("read file"); diff --git a/codex-rs/file-system/Cargo.toml b/codex-rs/file-system/Cargo.toml index 087e5da6c..d1beefbc8 100644 --- a/codex-rs/file-system/Cargo.toml +++ b/codex-rs/file-system/Cargo.toml @@ -9,7 +9,6 @@ workspace = true [dependencies] codex-protocol = { workspace = true } -codex-utils-absolute-path = { workspace = true } codex-utils-path-uri = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 58dcde604..929162b10 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -7,7 +7,6 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use std::future::Future; use std::io; @@ -51,7 +50,7 @@ pub struct ReadDirectoryEntry { pub struct FileSystemSandboxContext { pub permissions: PermissionProfile, #[serde(default, skip_serializing_if = "Option::is_none")] - pub cwd: Option, + pub cwd: Option, pub windows_sandbox_level: WindowsSandboxLevel, #[serde(default)] pub windows_sandbox_private_desktop: bool, @@ -60,32 +59,35 @@ pub struct FileSystemSandboxContext { } impl FileSystemSandboxContext { - pub fn from_legacy_sandbox_policy(sandbox_policy: SandboxPolicy, cwd: AbsolutePathBuf) -> Self { + pub fn from_legacy_sandbox_policy( + sandbox_policy: SandboxPolicy, + cwd: PathUri, + ) -> io::Result { + // Legacy policy projection materializes native roots, so convert at the receiving-host + // boundary while retaining the URI in the resulting sandbox context. + let native_cwd = cwd.to_abs_path()?; let file_system_sandbox_policy = - FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&sandbox_policy, &cwd); + FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( + &sandbox_policy, + &native_cwd, + ); let permissions = PermissionProfile::from_runtime_permissions_with_enforcement( SandboxEnforcement::from_legacy_sandbox_policy(&sandbox_policy), &file_system_sandbox_policy, NetworkSandboxPolicy::from(&sandbox_policy), ); - Self::from_permission_profile_with_cwd(permissions, cwd) + Ok(Self::from_permission_profile_with_cwd(permissions, cwd)) } pub fn from_permission_profile(permissions: PermissionProfile) -> Self { Self::from_permissions_and_cwd(permissions, /*cwd*/ None) } - pub fn from_permission_profile_with_cwd( - permissions: PermissionProfile, - cwd: AbsolutePathBuf, - ) -> Self { + pub fn from_permission_profile_with_cwd(permissions: PermissionProfile, cwd: PathUri) -> Self { Self::from_permissions_and_cwd(permissions, Some(cwd)) } - fn from_permissions_and_cwd( - permissions: PermissionProfile, - cwd: Option, - ) -> Self { + fn from_permissions_and_cwd(permissions: PermissionProfile, cwd: Option) -> Self { Self { permissions, cwd, diff --git a/codex-rs/sandboxing/Cargo.toml b/codex-rs/sandboxing/Cargo.toml index 226849521..ccdcd8768 100644 --- a/codex-rs/sandboxing/Cargo.toml +++ b/codex-rs/sandboxing/Cargo.toml @@ -16,6 +16,7 @@ workspace = true codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } dunce = { workspace = true } libc = { workspace = true } serde_json = { workspace = true } diff --git a/codex-rs/sandboxing/src/lib.rs b/codex-rs/sandboxing/src/lib.rs index 2dda65f9d..7210acd0d 100644 --- a/codex-rs/sandboxing/src/lib.rs +++ b/codex-rs/sandboxing/src/lib.rs @@ -33,6 +33,10 @@ pub fn system_bwrap_warning( impl From for CodexErr { fn from(err: SandboxTransformError) -> Self { match err { + error @ SandboxTransformError::InvalidCommandCwd { .. } + | error @ SandboxTransformError::InvalidSandboxPolicyCwd { .. } => { + CodexErr::InvalidRequest(error.to_string()) + } SandboxTransformError::MissingLinuxSandboxExecutable => { CodexErr::LandlockSandboxExecutableNotProvided } diff --git a/codex-rs/sandboxing/src/manager.rs b/codex-rs/sandboxing/src/manager.rs index 682e8e3dc..35ebaeac5 100644 --- a/codex-rs/sandboxing/src/manager.rs +++ b/codex-rs/sandboxing/src/manager.rs @@ -15,8 +15,10 @@ 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::PathUri; use std::collections::HashMap; use std::ffi::OsString; +use std::io; use std::path::Path; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -86,15 +88,20 @@ pub fn with_managed_mitm_ca_readable_root( pub struct SandboxCommand { pub program: OsString, pub args: Vec, - pub cwd: AbsolutePathBuf, + pub cwd: PathUri, pub env: HashMap, pub additional_permissions: Option, } +/// A host-native launch request produced after [`SandboxManager::transform`] validates URI inputs. +/// Build this only at the execution boundary: in exec-server, or in its logical equivalent within +/// app-server. Orchestration and transport code should retain [`PathUri`] values and defer +/// conversion to native paths until this request is created. #[derive(Debug)] pub struct SandboxExecRequest { pub command: Vec, pub cwd: AbsolutePathBuf, + pub sandbox_policy_cwd: AbsolutePathBuf, pub env: HashMap, pub network: Option, pub sandbox: SandboxType, @@ -117,7 +124,7 @@ pub struct SandboxTransformRequest<'a> { // TODO(viyatb): Evaluate switching this to Option> // to make shared ownership explicit across runtime/sandbox plumbing. pub network: Option<&'a NetworkProxy>, - pub sandbox_policy_cwd: &'a Path, + pub sandbox_policy_cwd: &'a PathUri, pub codex_linux_sandbox_exe: Option<&'a Path>, pub use_legacy_landlock: bool, pub windows_sandbox_level: WindowsSandboxLevel, @@ -126,6 +133,14 @@ pub struct SandboxTransformRequest<'a> { #[derive(Debug)] pub enum SandboxTransformError { + InvalidCommandCwd { + cwd: PathUri, + source: io::Error, + }, + InvalidSandboxPolicyCwd { + cwd: PathUri, + source: io::Error, + }, MissingLinuxSandboxExecutable, #[cfg(target_os = "linux")] Wsl1UnsupportedForBubblewrap, @@ -136,6 +151,16 @@ pub enum SandboxTransformError { impl std::fmt::Display for SandboxTransformError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { + Self::InvalidCommandCwd { cwd, source } => { + write!( + f, + "command cwd URI `{cwd}` is not valid on this host: {source}" + ) + } + Self::InvalidSandboxPolicyCwd { cwd, source } => write!( + f, + "sandbox policy cwd URI `{cwd}` is not valid on this host: {source}" + ), Self::MissingLinuxSandboxExecutable => { write!(f, "missing codex-linux-sandbox executable path") } @@ -147,7 +172,19 @@ impl std::fmt::Display for SandboxTransformError { } } -impl std::error::Error for SandboxTransformError {} +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::MissingLinuxSandboxExecutable => None, + #[cfg(target_os = "linux")] + Self::Wsl1UnsupportedForBubblewrap => None, + #[cfg(not(target_os = "macos"))] + Self::SeatbeltUnavailable => None, + } + } +} #[derive(Default)] pub struct SandboxManager; @@ -202,6 +239,18 @@ impl SandboxManager { windows_sandbox_level, windows_sandbox_private_desktop, } = request; + let native_command_cwd = command.cwd.to_abs_path().map_err(|source| { + SandboxTransformError::InvalidCommandCwd { + cwd: command.cwd.clone(), + source, + } + })?; + let native_sandbox_policy_cwd = sandbox_policy_cwd.to_abs_path().map_err(|source| { + SandboxTransformError::InvalidSandboxPolicyCwd { + cwd: sandbox_policy_cwd.clone(), + source, + } + })?; let additional_permissions = command.additional_permissions.take(); let managed_mitm_ca_trust_bundle_path = network.and_then(NetworkProxy::managed_mitm_ca_trust_bundle_path); @@ -210,7 +259,7 @@ impl SandboxManager { let effective_permission_profile = with_managed_mitm_ca_readable_root( effective_permission_profile, managed_mitm_ca_trust_bundle_path.as_ref(), - sandbox_policy_cwd, + native_sandbox_policy_cwd.as_path(), ); let (effective_file_system_policy, effective_network_policy) = effective_permission_profile.to_runtime_permissions(); @@ -230,7 +279,7 @@ impl SandboxManager { command: os_argv_to_strings(argv), file_system_sandbox_policy: &effective_file_system_policy, network_sandbox_policy: effective_network_policy, - sandbox_policy_cwd, + sandbox_policy_cwd: native_sandbox_policy_cwd.as_path(), enforce_managed_network, network, extra_allow_unix_sockets: &[], @@ -255,9 +304,9 @@ impl SandboxManager { )?; let mut args = create_linux_sandbox_command_args_for_permission_profile( os_argv_to_strings(argv), - command.cwd.as_path(), + native_command_cwd.as_path(), &effective_permission_profile, - sandbox_policy_cwd, + native_sandbox_policy_cwd.as_path(), use_legacy_landlock, allow_proxy_network, ); @@ -274,7 +323,8 @@ impl SandboxManager { Ok(SandboxExecRequest { command: argv, - cwd: command.cwd, + cwd: native_command_cwd, + sandbox_policy_cwd: native_sandbox_policy_cwd, env: command.env, network: network.cloned(), sandbox, diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 58138ad77..2cbcc483f 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -17,6 +17,7 @@ use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use dunce::canonicalize; use pretty_assertions::assert_eq; use std::collections::HashMap; @@ -74,6 +75,7 @@ fn restricted_file_system_uses_platform_sandbox_without_managed_network() { fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); + let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); let permissions = PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Restricted, @@ -83,7 +85,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() command: SandboxCommand { program: "true".into(), args: Vec::new(), - cwd: cwd.clone(), + cwd: cwd_uri.clone(), env: HashMap::new(), additional_permissions: None, }, @@ -91,7 +93,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() sandbox: SandboxType::None, enforce_managed_network: false, network: None, - sandbox_policy_cwd: cwd.as_path(), + sandbox_policy_cwd: &cwd_uri, codex_linux_sandbox_exe: None, use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, @@ -99,6 +101,8 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() }) .expect("transform"); + assert_eq!(exec_request.cwd, cwd); + assert_eq!(exec_request.sandbox_policy_cwd, cwd); assert_eq!( exec_request.file_system_sandbox_policy, FileSystemSandboxPolicy::unrestricted() @@ -113,6 +117,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() fn transform_additional_permissions_enable_network_for_external_sandbox() { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); + let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); let permissions = PermissionProfile::External { network: NetworkSandboxPolicy::Restricted, }; @@ -126,7 +131,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { command: SandboxCommand { program: "true".into(), args: Vec::new(), - cwd: cwd.clone(), + cwd: cwd_uri.clone(), env: HashMap::new(), additional_permissions: Some(AdditionalPermissionProfile { network: Some(NetworkPermissions { @@ -142,7 +147,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { sandbox: SandboxType::None, enforce_managed_network: false, network: None, - sandbox_policy_cwd: cwd.as_path(), + sandbox_policy_cwd: &cwd_uri, codex_linux_sandbox_exe: None, use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, @@ -166,6 +171,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { fn transform_additional_permissions_preserves_denied_entries() { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); + let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); let temp_dir = TempDir::new().expect("create temp dir"); let workspace_root = AbsolutePathBuf::from_absolute_path( canonicalize(temp_dir.path()).expect("canonicalize temp dir"), @@ -196,7 +202,7 @@ fn transform_additional_permissions_preserves_denied_entries() { command: SandboxCommand { program: "true".into(), args: Vec::new(), - cwd: cwd.clone(), + cwd: cwd_uri.clone(), env: HashMap::new(), additional_permissions: Some(AdditionalPermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( @@ -210,7 +216,7 @@ fn transform_additional_permissions_preserves_denied_entries() { sandbox: SandboxType::None, enforce_managed_network: false, network: None, - sandbox_policy_cwd: cwd.as_path(), + sandbox_policy_cwd: &cwd_uri, codex_linux_sandbox_exe: None, use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled, @@ -291,13 +297,14 @@ fn transform_linux_seccomp_request( ) -> super::SandboxExecRequest { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); + let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); let permissions = PermissionProfile::Disabled; manager .transform(SandboxTransformRequest { command: SandboxCommand { program: "true".into(), args: Vec::new(), - cwd: cwd.clone(), + cwd: cwd_uri.clone(), env: HashMap::new(), additional_permissions: None, }, @@ -305,7 +312,7 @@ fn transform_linux_seccomp_request( sandbox: SandboxType::LinuxSeccomp, enforce_managed_network: false, network: None, - sandbox_policy_cwd: cwd.as_path(), + sandbox_policy_cwd: &cwd_uri, codex_linux_sandbox_exe: Some(codex_linux_sandbox_exe), use_legacy_landlock: false, windows_sandbox_level: WindowsSandboxLevel::Disabled,