From 83dc7d11e873f43533dc898d50fae71cf4d55dc5 Mon Sep 17 00:00:00 2001 From: Sean Huang Date: Wed, 16 Sep 2026 04:11:18 +0000 Subject: [PATCH] Preserve executor path URIs in permission profile workspace roots (#45863) ## Why Executor profile roots can use path conventions that are not native to the current host. Converting them to host paths during configuration or turn reconstruction can reject or drop those roots, while case-insensitive comparison can hide Windows path spelling changes. ## What changed - Store profile roots as URI-backed `ProfileWorkspaceRoot` values throughout permission snapshots and thread settings, preserving spelling in equality and deduplication. - Keep effective workspace roots as `PathUri` values for permission materialization and status summaries. Convert Windows sandbox root hints to native paths only at native Windows sandbox boundaries, rejecting incompatible roots. - Omit the legacy rollout `workspace_roots` field when profile roots cannot be represented as host paths, retaining the compiled permission profile. ## Testing Add regression coverage for Windows and UNC root spelling changes, settings restoration and turn recording with foreign roots, executor-root status display, and backend-specific Windows root conversion. GitOrigin-RevId: 903c068fd74959bdd10e7cb1141aa42b953a59a4 --- .../windows_sandbox_processor.rs | 16 ++- .../windows_sandbox_setup_config_tests.rs | 6 +- codex-rs/cli/src/debug_sandbox.rs | 9 +- codex-rs/core/src/codex_thread.rs | 5 +- codex-rs/core/src/config/config_tests.rs | 8 +- codex-rs/core/src/config/mod.rs | 27 +++-- codex-rs/core/src/config/permissions.rs | 2 +- .../src/config/resolved_permission_profile.rs | 12 +- codex-rs/core/src/environment_selection.rs | 6 +- codex-rs/core/src/exec.rs | 28 ++++- codex-rs/core/src/exec_tests.rs | 89 ++++++++++----- codex-rs/core/src/session/session.rs | 5 +- codex-rs/core/src/session/tests.rs | 4 +- codex-rs/core/src/session/turn_context.rs | 42 +++++-- codex-rs/core/src/tasks/user_shell.rs | 2 +- codex-rs/core/src/unified_exec/mod_tests.rs | 7 +- codex-rs/core/tests/suite/remote_env.rs | 106 +++++++++++++++++- .../src/event_processor_with_human_output.rs | 6 +- codex-rs/exec/tests/suite/sandbox.rs | 3 +- codex-rs/protocol/src/models.rs | 1 + .../src/permission_profile_snapshot.rs | 48 +++++++- .../src/permission_profile_snapshot_tests.rs | 20 ++++ codex-rs/protocol/src/protocol.rs | 3 +- codex-rs/tui/src/app/thread_routing.rs | 4 +- .../tui/src/chatwidget/status_surfaces.rs | 6 +- codex-rs/tui/src/status/card.rs | 6 +- codex-rs/tui/src/status/tests.rs | 42 ++++++- 27 files changed, 408 insertions(+), 105 deletions(-) create mode 100644 codex-rs/protocol/src/permission_profile_snapshot_tests.rs 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)" + ); +}