diff --git a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs index 3c45b33125..8fd1859967 100644 --- a/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs +++ b/codex-rs/app-server/src/request_processors/windows_sandbox_processor.rs @@ -1,6 +1,7 @@ use super::*; #[cfg(target_os = "windows")] use anyhow::Context as _; +use codex_utils_path_uri::PathUri; #[derive(Clone)] pub(crate) struct WindowsSandboxRequestProcessor { @@ -113,6 +114,19 @@ impl WindowsSandboxRequestProcessor { params.mode, )?; + // Provisioning installs native Windows filesystem permissions. Validate + // this boundary before acknowledging that setup has started. + let workspace_roots = config + .effective_workspace_roots() + .iter() + .map(PathUri::to_abs_path) + .collect::>>() + .map_err(|err| { + invalid_request(format!( + "workspace roots are not native to this host: {err}" + )) + })?; + self.outgoing .send_response( request_id.clone(), @@ -127,7 +141,7 @@ impl WindowsSandboxRequestProcessor { let setup_request = WindowsSandboxSetupRequest { mode: setup_mode, permission_profile: config.permissions.effective_permission_profile(), - workspace_roots: config.effective_workspace_roots(), + workspace_roots, command_cwd, env_map: std::env::vars().collect(), codex_home: config.codex_home.to_path_buf(), diff --git a/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs b/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs index 073c947af9..a6fef09638 100644 --- a/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs +++ b/codex-rs/app-server/src/request_processors/windows_sandbox_setup_config_tests.rs @@ -3,6 +3,7 @@ use super::ConfigManager; use super::load_setup_config; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; #[tokio::test] @@ -32,7 +33,10 @@ async fn explicit_cwd_remains_the_setup_workspace() -> anyhow::Result<()> { assert_eq!( (command_cwd, config.effective_workspace_roots()), - (project.path().to_path_buf(), vec![expected]), + ( + project.path().to_path_buf(), + vec![PathUri::from_abs_path(&expected)], + ), ); Ok(()) } diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index aea39b94f2..f216e4df2e 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -254,9 +254,6 @@ async fn run_command_under_sandbox( // remaining cwd-dependent policy resolution. `:workspace_roots` entries in // the effective profile have already been materialized from config roots. let sandbox_policy_cwd = cwd.clone(); - #[cfg(target_os = "windows")] - let workspace_roots = config.effective_workspace_roots(); - let env = create_env( &config.permissions.shell_environment_policy, /*thread_id*/ None, @@ -323,6 +320,12 @@ async fn run_command_under_sandbox( { if config.permissions.windows_sandbox_type != codex_sandboxing::SandboxType::WindowsMxc { + let workspace_roots = config + .effective_workspace_roots() + .iter() + .map(codex_utils_path_uri::PathUri::to_abs_path) + .collect::>>() + .context("invalid Windows sandbox workspace roots")?; run_command_under_windows_session( &config, &permission_profile, diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index d2bbbe84f2..15f36cb12a 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -29,6 +29,7 @@ use codex_protocol::mcp::CallToolResult; use codex_protocol::mcp::ClientMcpExtensions; use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::ProfileWorkspaceRoot; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::protocol::AskForApproval; @@ -91,7 +92,7 @@ pub struct ThreadConfigSnapshot { pub active_permission_profile: Option, pub environments: TurnEnvironmentSelections, pub workspace_roots: Vec, - pub profile_workspace_roots: Vec, + pub profile_workspace_roots: Vec, pub ephemeral: bool, pub reasoning_effort: Option, pub reasoning_summary: Option, @@ -140,7 +141,7 @@ impl ThreadConfigSnapshot { pub struct CodexThreadSettingsOverrides { pub environments: Option, pub runtime_workspace_roots: Option>, - pub profile_workspace_roots: Option>, + pub profile_workspace_roots: Option>, pub approval_policy: Option, pub approvals_reviewer: Option, pub sandbox_policy: Option, diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index d4e1adf178..718e06f6bf 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -3174,9 +3174,9 @@ async fn workspace_profile_applies_rules_to_runtime_and_profile_workspace_roots( assert_eq!( config.effective_workspace_roots(), vec![ - cwd_abs.clone(), - runtime_root_abs.clone(), - profile_root_abs.clone() + PathUri::from_abs_path(&cwd_abs), + PathUri::from_abs_path(&runtime_root_abs), + PathUri::from_abs_path(&profile_root_abs), ] ); @@ -3197,7 +3197,7 @@ async fn workspace_profile_applies_rules_to_runtime_and_profile_workspace_roots( } assert_eq!( config.permissions.profile_workspace_roots(), - std::slice::from_ref(&profile_root_abs) + &[profile_root_abs.into()] ); assert_eq!( config.permissions.active_permission_profile(), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 789b207da3..23a6f83103 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -114,6 +114,7 @@ use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::BaseInstructionsProvenance; use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::PermissionProfileSnapshot; +use codex_protocol::models::ProfileWorkspaceRoot; use codex_protocol::models::SandboxEnforcement; use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ModelsResponse; @@ -442,7 +443,7 @@ impl Permissions { &self.workspace_roots } - pub fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + pub fn profile_workspace_roots(&self) -> &[ProfileWorkspaceRoot] { self.permission_profile_state.profile_workspace_roots() } @@ -1604,10 +1605,22 @@ impl Config { Ok(()) } - pub fn effective_workspace_roots(&self) -> Vec { - let mut workspace_roots = self.workspace_roots.clone(); - workspace_roots.extend(self.permissions.profile_workspace_roots().iter().cloned()); - dedupe_absolute_paths(&mut workspace_roots); + /// Combine runtime and profile roots without interpreting them on the current host. + pub fn effective_workspace_roots(&self) -> Vec { + let mut workspace_roots = self + .workspace_roots + .iter() + .map(PathUri::from_abs_path) + .collect::>(); + workspace_roots.extend( + self.permissions + .profile_workspace_roots() + .iter() + .map(|root| root.as_uri().clone()), + ); + // Preserve spelling changes even when Windows path comparison folds case. + let mut seen = HashSet::new(); + workspace_roots.retain(|root| seen.insert(root.to_string())); workspace_roots } @@ -3540,10 +3553,6 @@ impl Config { &mut startup_warnings, )?; let file_system_sandbox_policy = permission_profile.file_system_sandbox_policy(); - let configured_workspace_roots = configured_workspace_roots - .iter() - .map(PathUri::to_abs_path) - .collect::>>()?; let active_permission_profile = if using_implicit_builtin_profile && default_permissions == BUILT_IN_WORKSPACE_PROFILE && cfg.sandbox_workspace_write.is_some() diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index 75612b2806..9cef5b075a 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -372,7 +372,7 @@ pub struct CompiledPermissionProfile { /// Resolves a selected profile and its roots using only supplied execution-host facts. /// Configured roots are materialized before return; symbolic roots remain available -/// for runtime workspace selection. Local callers only convert the returned URI types. +/// for runtime workspace selection. Callers retain URI roots until a native executor boundary. pub fn compile_permission_profile( permissions: Option<&PermissionsToml>, profile_name: &str, diff --git a/codex-rs/core/src/config/resolved_permission_profile.rs b/codex-rs/core/src/config/resolved_permission_profile.rs index ba61c765b2..dca30f1e32 100644 --- a/codex-rs/core/src/config/resolved_permission_profile.rs +++ b/codex-rs/core/src/config/resolved_permission_profile.rs @@ -3,7 +3,8 @@ use codex_config::ConstraintResult; use codex_protocol::models::ActivePermissionProfile; use codex_protocol::models::PermissionProfile; use codex_protocol::models::PermissionProfileSnapshot; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_protocol::models::ProfileWorkspaceRoot; +use codex_utils_path_uri::PathUri; #[derive(Debug, Clone, PartialEq)] pub(crate) struct PermissionProfileState { @@ -22,7 +23,7 @@ impl PermissionProfileState { pub(crate) fn from_constrained_active_profile( constrained_permission_profile: Constrained, active_permission_profile: Option, - profile_workspace_roots: Vec, + profile_workspace_roots: Vec, ) -> ConstraintResult { let permission_profile = constrained_permission_profile.get().clone(); let snapshot = match active_permission_profile { @@ -30,7 +31,10 @@ impl PermissionProfileState { PermissionProfileSnapshot::active_with_profile_workspace_roots( permission_profile, active_permission_profile, - profile_workspace_roots, + profile_workspace_roots + .into_iter() + .map(ProfileWorkspaceRoot::from) + .collect(), ) } None => PermissionProfileSnapshot::legacy(permission_profile), @@ -60,7 +64,7 @@ impl PermissionProfileState { self.permission_profile.get().active_permission_profile() } - pub(crate) fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + pub(crate) fn profile_workspace_roots(&self) -> &[ProfileWorkspaceRoot] { self.permission_profile.get().profile_workspace_roots() } diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 3af81b3286..c1993ab5fc 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -1297,7 +1297,7 @@ url = "ws://127.0.0.1:8765" permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), ActivePermissionProfile::read_only(), - vec![cwd.join("profile-root")], + vec![cwd.join("profile-root").into()], ), shell_environment_policy: Default::default(), exec_policy: None, @@ -1562,7 +1562,7 @@ url = "ws://127.0.0.1:8765" permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), ActivePermissionProfile::read_only(), - vec![cwd.join("profile-root")], + vec![cwd.join("profile-root").into()], ), shell_environment_policy: Default::default(), exec_policy: None, @@ -1944,7 +1944,7 @@ url = "ws://127.0.0.1:8765" permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), ActivePermissionProfile::read_only(), - vec![cwd.join("child-profile-root")], + vec![cwd.join("child-profile-root").into()], ), shell_environment_policy: Default::default(), exec_policy: None, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 2b47786397..0144705f6a 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -308,11 +308,15 @@ pub async fn process_exec_tool_call( use_legacy_landlock: bool, stdout_stream: Option, ) -> Result { + let windows_sandbox_workspace_roots = windows_sandbox_workspace_roots + .iter() + .map(PathUri::from_abs_path) + .collect::>(); let exec_req = build_exec_request( params, permission_profile, sandbox_cwd, - windows_sandbox_workspace_roots, + &windows_sandbox_workspace_roots, codex_linux_sandbox_exe, codex_self_exe, use_legacy_landlock, @@ -328,7 +332,7 @@ pub fn build_exec_request( params: ExecParams, permission_profile: &PermissionProfile, sandbox_cwd: &AbsolutePathBuf, - windows_sandbox_workspace_roots: &[AbsolutePathBuf], + windows_sandbox_workspace_roots: &[PathUri], codex_linux_sandbox_exe: &Option, codex_self_exe: &Option, use_legacy_landlock: bool, @@ -407,10 +411,24 @@ pub fn build_exec_request( windows_sandbox_private_desktop, }) .map_err(CodexErr::from)?; - let windows_sandbox_workspace_roots = if windows_sandbox_workspace_roots.is_empty() { - vec![sandbox_cwd.clone()] + // These hints belong to the native Windows backend. Other backends use + // the materialized profile and must not project executor paths onto this host. + let windows_sandbox_workspace_roots = if sandbox_type == SandboxType::WindowsRestrictedToken { + if windows_sandbox_workspace_roots.is_empty() { + vec![sandbox_cwd.clone()] + } else { + windows_sandbox_workspace_roots + .iter() + .map(PathUri::to_abs_path) + .collect::>>() + .map_err(|err| { + CodexErr::InvalidRequest(format!( + "invalid Windows sandbox workspace roots: {err}" + )) + })? + } } else { - windows_sandbox_workspace_roots.to_vec() + Vec::new() }; ExecRequest::from_sandbox_exec_request(request, options, windows_sandbox_workspace_roots) } diff --git a/codex-rs/core/src/exec_tests.rs b/codex-rs/core/src/exec_tests.rs index 93000cfb51..dabafb8e97 100644 --- a/codex-rs/core/src/exec_tests.rs +++ b/codex-rs/core/src/exec_tests.rs @@ -1195,39 +1195,72 @@ fn process_exec_tool_call_uses_platform_sandbox_for_network_only_restrictions() } #[test] -fn build_exec_request_preserves_windows_workspace_roots() -> Result<()> { +fn build_exec_request_projects_workspace_roots_only_for_windows_sandbox() -> Result<()> { let temp_dir = tempfile::TempDir::new()?; let cwd = temp_dir.path().abs(); - let additional_root = temp_dir.path().join("additional").abs(); - let workspace_roots = vec![cwd.clone(), additional_root]; - - let exec_request = build_exec_request( - ExecParams { - command: vec!["echo".to_string(), "ok".to_string()], - cwd: cwd.clone(), - expiration: ExecExpiration::DefaultTimeout, - capture_policy: ExecCapturePolicy::ShellTool, - env: HashMap::new(), - network: None, - network_environment_id: None, - sandbox_permissions: SandboxPermissions::UseDefault, - windows_sandbox_level: WindowsSandboxLevel::Disabled, - windows_sandbox_private_desktop: false, - justification: None, - arg0: None, - }, - &PermissionProfile::Disabled, - &cwd, - workspace_roots.as_slice(), - &None, - /*codex_self_exe*/ &None, - /*use_legacy_landlock*/ false, + let build_request = |profile: &PermissionProfile, roots: &[PathUri]| { + build_exec_request( + ExecParams { + command: vec!["echo".to_string(), "ok".to_string()], + cwd: cwd.clone(), + expiration: ExecExpiration::DefaultTimeout, + capture_policy: ExecCapturePolicy::ShellTool, + env: HashMap::new(), + network: None, + network_environment_id: None, + sandbox_permissions: SandboxPermissions::UseDefault, + windows_sandbox_level: WindowsSandboxLevel::RestrictedToken, + windows_sandbox_private_desktop: false, + justification: None, + arg0: None, + }, + profile, + &cwd, + roots, + &Some(temp_dir.path().join("codex-linux-sandbox")), + /*codex_self_exe*/ &None, + /*use_legacy_landlock*/ false, + ) + }; + let native_roots = vec![cwd.clone(), temp_dir.path().join("additional").abs()]; + let request = build_request( + &PermissionProfile::read_only(), + &native_roots + .iter() + .map(PathUri::from_abs_path) + .collect::>(), )?; - assert_eq!( - exec_request.windows_sandbox_workspace_roots, - workspace_roots + request.windows_sandbox_workspace_roots, + if cfg!(windows) { + native_roots + } else { + Vec::new() + }, ); + + let foreign_root = PathUri::parse(if cfg!(windows) { + "file:///workspace" + } else { + "file:///C:/workspace" + }) + .expect("foreign workspace URI"); + let roots = [PathUri::from_abs_path(&cwd), foreign_root]; + assert_eq!( + build_request(&PermissionProfile::Disabled, &roots)?.windows_sandbox_workspace_roots, + Vec::new(), + ); + let request = build_request(&PermissionProfile::read_only(), &roots); + if cfg!(windows) { + let error = request.expect_err("native Windows sandbox must reject foreign roots"); + assert!(matches!( + error.details(), + codex_protocol::error::CodexErrorDetails::InvalidRequest(message) + if message.starts_with("invalid Windows sandbox workspace roots:") + )); + } else { + assert_eq!(request?.windows_sandbox_workspace_roots, Vec::new()); + } Ok(()) } diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index e5e96e32b4..c508c9580d 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -31,6 +31,7 @@ use codex_protocol::SessionId; use codex_protocol::capabilities::SelectedCapabilityRoot; use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::mcp::ClientMcpExtensions; +use codex_protocol::models::ProfileWorkspaceRoot; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::EnvironmentConfig; @@ -545,7 +546,7 @@ impl SessionConfiguration { &mut self, permission_profile: PermissionProfile, active_permission_profile: Option, - profile_workspace_roots: Vec, + profile_workspace_roots: Vec, preserve_deny_reads_from: Option<&FileSystemSandboxPolicy>, ) -> ConstraintResult<()> { let enforcement = permission_profile.enforcement(); @@ -589,7 +590,7 @@ pub(crate) struct SessionSettingsUpdate { pub(crate) step_settings: StepSettingsUpdate, pub(crate) environments: Option, pub(crate) runtime_workspace_roots: Option>, - pub(crate) profile_workspace_roots: Option>, + pub(crate) profile_workspace_roots: Option>, pub(crate) sandbox_policy: Option, pub(crate) permission_profile: Option, pub(crate) active_permission_profile: Option, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 745756d48a..aee5d07e5b 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5606,7 +5606,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() { let updates = SessionSettingsUpdate { permission_profile: Some(PermissionProfile::read_only()), active_permission_profile: Some(active_profile.clone()), - profile_workspace_roots: Some(vec![profile_root.clone()]), + profile_workspace_roots: Some(vec![profile_root.clone().into()]), ..Default::default() }; @@ -5636,7 +5636,7 @@ async fn permission_profile_updates_apply_to_next_turn_environment() { PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), active_profile, - vec![profile_root], + vec![profile_root.into()], ); assert_eq!(next_environment.config(), &expected_environment_config); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index e5a21b6e8e..ae88a809ae 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -480,22 +480,22 @@ impl TurnContext { } /// Combines the selected environment's workspace roots with its permission profile roots. - pub(crate) fn effective_workspace_roots(&self) -> Vec { + pub(crate) fn effective_workspace_roots(&self) -> Vec { let Some(environment) = self.environments.primary() else { return self.config.effective_workspace_roots(); }; - let mut workspace_roots = environment - .workspace_roots() - .iter() - .filter_map(|root| root.to_abs_path().ok()) - .collect::>(); + let mut workspace_roots = environment.workspace_roots().to_vec(); for root in environment .config() .permission_profile .profile_workspace_roots() { - if !workspace_roots.contains(root) { + let root = root.as_uri(); + if !workspace_roots + .iter() + .any(|existing| existing.to_string() == root.to_string()) + { workspace_roots.push(root.clone()); } } @@ -652,7 +652,31 @@ impl TurnContext { } pub(crate) fn to_turn_context_item(&self) -> TurnContextItem { - let workspace_roots = self.effective_workspace_roots(); + // The legacy rollout field still stores host-native paths. Keep its + // runtime-root filtering and omit it for unrepresentable profile roots; + // the authoritative permission profile retains its concrete entries. + let profile_roots = self.environments.primary().map_or_else( + || self.config.permissions.profile_workspace_roots(), + |environment| { + environment + .config() + .permission_profile + .profile_workspace_roots() + }, + ); + let workspace_roots = if profile_roots + .iter() + .all(|root| root.as_uri().to_abs_path().is_ok()) + { + let roots = self + .effective_workspace_roots() + .iter() + .filter_map(|root| root.to_abs_path().ok()) + .collect::>(); + (!roots.is_empty()).then_some(roots) + } else { + None + }; #[allow(deprecated)] let cwd = self.cwd.clone(); TurnContextItem { @@ -660,7 +684,7 @@ impl TurnContext { root_turn_id: self.turn_metadata_state.root_turn_id(), disabled_plugin_ids: Some(self.disabled_plugin_ids.clone()), cwd, - workspace_roots: (!workspace_roots.is_empty()).then_some(workspace_roots), + workspace_roots, current_date: self.current_date.clone(), timezone: self.timezone.clone(), approval_policy: self.approval_policy(), diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index a38f26d200..8741ec4856 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -224,7 +224,7 @@ pub(crate) async fn execute_user_shell_command( capture_policy: ExecCapturePolicy::ShellTool, sandbox: SandboxType::None, windows_sandbox_policy_cwd: cwd.clone().into(), - windows_sandbox_workspace_roots: turn_context.effective_workspace_roots(), + windows_sandbox_workspace_roots: Vec::new(), windows_sandbox_level: turn_context.windows_sandbox_level, windows_sandbox_private_desktop: turn_context .config diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index a6f488689b..027c986ad9 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -81,7 +81,12 @@ fn test_exec_request( ExecExpiration::DefaultTimeout, ExecCapturePolicy::ShellTool, SandboxType::None, - turn.config.effective_workspace_roots(), + turn.config + .effective_workspace_roots() + .iter() + .map(PathUri::to_abs_path) + .collect::>>() + .expect("test workspace roots are host-native"), turn.windows_sandbox_level, windows_sandbox_private_desktop, permission_profile, diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index eada6c37ba..78c3d5ed3b 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -506,7 +506,7 @@ async fn environment_permissions_follow_configuration_ownership() -> Result<()> let owner_permission_profile = PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), owner_active_profile.clone(), - vec![owner_profile_workspace_root.clone()], + vec![owner_profile_workspace_root.clone().into()], ); let (shell, command) = match test_target_os() { @@ -635,7 +635,7 @@ async fn environment_permissions_follow_configuration_ownership() -> Result<()> ); assert_eq!( snapshot.profile_workspace_roots, - vec![owner_profile_workspace_root.clone()] + vec![owner_profile_workspace_root.clone().into()] ); assert_eq!( persisted_settings, @@ -705,6 +705,104 @@ async fn environment_permissions_follow_configuration_ownership() -> Result<()> Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn executor_profile_roots_survive_settings_restore_and_turn_recording() -> Result<()> { + let server = start_mock_server().await; + let test = test_codex() + .with_config(|config| { + config.workspace_roots = vec![config.cwd.clone()]; + config + .permissions + .set_workspace_roots(config.workspace_roots.clone()); + }) + .build_with_auto_env(&server) + .await?; + let runtime_roots = test.codex.config_snapshot().await.workspace_roots; + // Include a foreign convention on every host, plus case-distinct Windows roots. + let roots = [ + "file:///workspace/profile", + "file:///C:/Work/Profile", + "file:///C:/work/profile", + "file://server/share/profile", + ] + .map(|root| PathUri::parse(root).unwrap()); + let profile = PermissionProfile::workspace_write_with_path_uris( + &roots, + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ); + let expected_profile = profile.clone().materialize_project_roots_with_path_uris( + &test.executor_environment().selection().workspace_roots, + ); + let profile_roots = roots.into_iter().map(Into::into).collect::>(); + let active_profile = ActivePermissionProfile::new("executor"); + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + permission_profile: Some(profile.clone()), + active_permission_profile: Some(active_profile.clone()), + profile_workspace_roots: Some(profile_roots.clone()), + ..Default::default() + }, + ) + .await?; + let saved = test.codex.restorable_thread_settings().await; + assert_eq!(saved.profile_workspace_roots, Some(profile_roots.clone())); + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + permission_profile: Some(profile), + profile_workspace_roots: Some(Vec::new()), + ..Default::default() + }, + ) + .await?; + assert!( + test.codex + .config_snapshot() + .await + .profile_workspace_roots + .is_empty() + ); + test.codex.restore_thread_settings(saved).await?; + let snapshot = test.codex.config_snapshot().await; + assert_eq!( + (snapshot.profile_workspace_roots, snapshot.workspace_roots), + (profile_roots, runtime_roots) + ); + + let response_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + test.submit_text_turn("record the executor profile").await?; + response_mock.single_request(); + test.codex.flush_rollout().await?; + let context = test + .codex + .load_history(/*include_archived*/ false) + .await? + .items + .into_iter() + .find_map(|item| match item { + RolloutItem::TurnContext(context) => Some(context), + _ => None, + }) + .context("recorded turn context")?; + // The rollout retains compiled permissions, not the unprojectable root list. + assert_eq!( + ( + context.permission_profile, + context.active_permission_profile, + context.workspace_roots + ), + (Some(expected_profile), Some(active_profile), None) + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn step_world_state_gates_deferred_prompt_independently_of_host_config() -> Result<()> { for deferred_executor_enabled in [false, true] { @@ -2322,7 +2420,7 @@ async fn deferred_executor_spawn_agent_inherits_ready_step_environments( permission_profile: PermissionProfileSnapshot::active_with_profile_workspace_roots( PermissionProfile::read_only(), owner_active_profile.clone(), - vec![owner_profile_workspace_root.clone()], + vec![owner_profile_workspace_root.clone().into()], ), shell_environment_policy: Default::default(), windows_sandbox_level: WindowsSandboxLevel::from_config(&test.config), @@ -2386,7 +2484,7 @@ async fn deferred_executor_spawn_agent_inherits_ready_step_environments( ( PermissionProfile::read_only(), Some(owner_active_profile), - vec![owner_profile_workspace_root], + vec![owner_profile_workspace_root.into()], ) ); assert!( diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 1492502455..222bc485ab 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -446,11 +446,7 @@ fn config_summary_entries( summarize_permission_profile( &permission_profile, &PathUri::from_abs_path(&config.cwd), - &config - .effective_workspace_roots() - .iter() - .map(PathUri::from_abs_path) - .collect::>(), + &config.effective_workspace_roots(), ), ), ]; diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 285cb73ae5..1ccaee5849 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -4,6 +4,7 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; +use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::future::Future; use std::io; @@ -50,7 +51,7 @@ pub(super) async fn spawn_command_under_sandbox( }, permission_profile, sandbox_cwd, - std::slice::from_ref(sandbox_cwd), + &[PathUri::from_abs_path(sandbox_cwd)], &codex_linux_sandbox_exe, /*codex_self_exe*/ &None, /*use_legacy_landlock*/ false, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 549d69d553..ee7fecbf06 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -39,6 +39,7 @@ mod item_metadata; pub use crate::local_media::MAX_PROMPT_AUDIO_INPUT_BYTES; pub use crate::local_media::snapshot_local_user_input; pub use crate::permission_profile_snapshot::PermissionProfileSnapshot; +pub use crate::permission_profile_snapshot::ProfileWorkspaceRoot; pub use configuration_update::ConfigurationReasoning; pub use executed_tool_calls::ExecutedToolCall; pub use executed_tool_calls::ExecutedToolCallArguments; diff --git a/codex-rs/protocol/src/permission_profile_snapshot.rs b/codex-rs/protocol/src/permission_profile_snapshot.rs index 0f97cc202d..34fe6fdb11 100644 --- a/codex-rs/protocol/src/permission_profile_snapshot.rs +++ b/codex-rs/protocol/src/permission_profile_snapshot.rs @@ -1,8 +1,44 @@ +//! Trusted permission state keeps local and remote profile roots as executor path URIs. + use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use crate::models::ActivePermissionProfile; use crate::models::PermissionProfile; +/// A profile-declared root, separate from turn-scoped runtime workspace roots. +/// +/// Local and remote roots share the same URI representation. Snapshot equality +/// compares the stored URI spelling without folding Windows case, so a spelling +/// change still propagates through settings. +#[derive(Debug, Clone, Eq)] +pub struct ProfileWorkspaceRoot(PathUri); + +impl PartialEq for ProfileWorkspaceRoot { + fn eq(&self, other: &Self) -> bool { + self.0.to_string() == other.0.to_string() + } +} + +impl ProfileWorkspaceRoot { + /// Borrow the resolved URI without projecting it onto the current host. + pub fn as_uri(&self) -> &PathUri { + &self.0 + } +} + +impl From for ProfileWorkspaceRoot { + fn from(path: AbsolutePathBuf) -> Self { + Self(PathUri::from_abs_path(&path)) + } +} + +impl From for ProfileWorkspaceRoot { + fn from(path: PathUri) -> Self { + Self(path) + } +} + /// Trusted snapshot of a resolved permission profile. /// /// Keeps the concrete permissions, optional active profile identity, and @@ -13,7 +49,7 @@ use crate::models::PermissionProfile; pub struct PermissionProfileSnapshot { permission_profile: PermissionProfile, active_permission_profile: Option, - profile_workspace_roots: Vec, + profile_workspace_roots: Vec, } impl PermissionProfileSnapshot { @@ -44,10 +80,12 @@ impl PermissionProfileSnapshot { /// Create a snapshot for an active profile and its declared roots. /// /// Profile roots remain distinct from turn-scoped runtime workspace roots. + /// The supplied profile must already contain their compiled permissions; + /// storing roots here does not itself grant access or materialize rules. pub fn active_with_profile_workspace_roots( permission_profile: PermissionProfile, active_permission_profile: ActivePermissionProfile, - profile_workspace_roots: Vec, + profile_workspace_roots: Vec, ) -> Self { Self { permission_profile, @@ -80,7 +118,11 @@ impl PermissionProfileSnapshot { } /// Borrow profile-declared workspace roots captured in this snapshot. - pub fn profile_workspace_roots(&self) -> &[AbsolutePathBuf] { + pub fn profile_workspace_roots(&self) -> &[ProfileWorkspaceRoot] { &self.profile_workspace_roots } } + +#[cfg(test)] +#[path = "permission_profile_snapshot_tests.rs"] +mod tests; diff --git a/codex-rs/protocol/src/permission_profile_snapshot_tests.rs b/codex-rs/protocol/src/permission_profile_snapshot_tests.rs new file mode 100644 index 0000000000..efe6e3ea15 --- /dev/null +++ b/codex-rs/protocol/src/permission_profile_snapshot_tests.rs @@ -0,0 +1,20 @@ +//! Profile-root spelling changes must propagate through settings snapshots. + +use super::*; + +#[test] +fn executor_profile_root_case_changes_update_snapshot() { + for (first, second) in [ + ("file:///C:/Work/Project", "file:///C:/work/project"), + ("file://server/share/Project", "file://server/share/project"), + ] { + let snapshot = |root| { + PermissionProfileSnapshot::active_with_profile_workspace_roots( + PermissionProfile::read_only(), + ActivePermissionProfile::new("executor"), + vec![PathUri::parse(root).unwrap().into()], + ) + }; + assert_ne!(snapshot(first), snapshot(second)); + } +} diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4f38880b9f..355181ff69 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -47,6 +47,7 @@ use crate::models::ImageDetail; use crate::models::InternalChatMessageMetadataPassthrough; use crate::models::MessagePhase; use crate::models::PermissionProfile; +use crate::models::ProfileWorkspaceRoot; use crate::models::ResponseInputItem; use crate::models::ResponseItem; use crate::models::SandboxEnforcement; @@ -528,7 +529,7 @@ pub struct ThreadSettingsOverrides { /// Updated profile-defined workspace roots for status summaries and /// per-turn config reconstruction. - pub profile_workspace_roots: Option>, + pub profile_workspace_roots: Option>, /// Updated command approval policy. pub approval_policy: Option, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index bebf0c04b0..b9dc907025 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -1023,9 +1023,7 @@ impl App { runtime_permission_profile_override.map(|profile| { profile .clone() - .materialize_project_roots_with_workspace_roots( - &config.effective_workspace_roots(), - ) + .materialize_project_roots_with_path_uris(&config.effective_workspace_roots()) }); if runtime_permission_profile_override .as_ref() diff --git a/codex-rs/tui/src/chatwidget/status_surfaces.rs b/codex-rs/tui/src/chatwidget/status_surfaces.rs index dd45d1b1c4..5424267870 100644 --- a/codex-rs/tui/src/chatwidget/status_surfaces.rs +++ b/codex-rs/tui/src/chatwidget/status_surfaces.rs @@ -1203,11 +1203,7 @@ fn permissions_display(config: &Config) -> String { let summary = summarize_permission_profile( &permission_profile, &PathUri::from_abs_path(&config.cwd), - &config - .effective_workspace_roots() - .iter() - .map(PathUri::from_abs_path) - .collect::>(), + &config.effective_workspace_roots(), ); if let Some(details) = summary.strip_prefix("read-only") && !details.contains("(network access enabled)") diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index c2ba3f8c52..ad79dcf6f2 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -298,11 +298,7 @@ impl StatusHistoryCell { ) -> Self { let approval_policy = AskForApproval::from(config.permissions.approval_policy.value()); let permission_profile = config.permissions.effective_permission_profile(); - let workspace_roots = config - .effective_workspace_roots() - .iter() - .map(PathUri::from_abs_path) - .collect::>(); + let workspace_roots = config.effective_workspace_roots(); let cwd = PathUri::from_abs_path(&config.cwd); let model_provider = model_provider_id .filter(|id| !id.trim().is_empty()) diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index 41069d770a..f9384badc9 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -52,6 +52,7 @@ use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::prelude::*; @@ -244,6 +245,10 @@ fn reset_at_from(captured_at: &chrono::DateTime, seconds: i64) -> } fn permissions_text_for(config: &Config) -> Option { + permissions_text_for_width(config, /*width*/ 80) +} + +fn permissions_text_for_width(config: &Config, width: u16) -> Option { let usage = TokenUsage::default(); let captured_at = chrono::Local .with_ymd_and_hms(2024, 1, 2, 3, 4, 5) @@ -265,7 +270,7 @@ fn permissions_text_for(config: &Config) -> Option { /*collaboration_mode*/ None, /*reasoning_effort_override*/ None, ); - render_lines(&composite.display_lines(/*width*/ 80)) + render_lines(&composite.display_lines(width)) .iter() .find(|line| line.contains("Permissions:")) .and_then(|line| { @@ -626,7 +631,7 @@ async fn status_permissions_workspace_roots_include_profile_defined_directories( /*exclude_slash_tmp*/ false, ), ActivePermissionProfile::new(":workspace"), - vec![profile_root.clone()], + vec![profile_root.clone().into()], ), ) .expect("set permission profile"); @@ -2225,3 +2230,36 @@ async fn status_context_window_uses_last_usage() { "context line should not use total aggregated tokens, got: {context_line}" ); } + +#[tokio::test] +async fn status_permissions_include_executor_profile_root() { + let temp_home = TempDir::new().expect("temp home"); + let mut config = test_config(&temp_home).await; + set_workspace_cwd(&mut config, test_path_buf("/workspace/repo").abs()); + config + .permissions + .approval_policy + .set(AskForApproval::OnRequest.to_core()) + .expect("set approval policy"); + let root = PathUri::parse("file://server/share/foreign").expect("executor URI"); + config + .permissions + .set_permission_profile_from_session_snapshot( + PermissionProfileSnapshot::active_with_profile_workspace_roots( + PermissionProfile::workspace_write_with_path_uris( + std::slice::from_ref(&root), + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ), + ActivePermissionProfile::new("executor"), + vec![root.into()], + ), + ) + .expect("set permission snapshot"); + + assert_snapshot!( + permissions_text_for_width(&config, /*width*/ 160).expect("permissions line"), + @r"Profile executor (workspace [\\server\share\foreign], Ask for approval)" + ); +}