diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 760969fbfe..757328f641 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3210,6 +3210,7 @@ dependencies = [ "codex-utils-cargo-bin", "codex-utils-cli", "codex-utils-oss", + "codex-utils-path-uri", "codex-utils-sandbox-summary", "codex-worktree", "core_test_support", @@ -5094,6 +5095,7 @@ version = "0.0.0" dependencies = [ "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "pretty_assertions", ] diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index a8ced9cdd7..42a75a0532 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -42,6 +42,7 @@ codex-rollout = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cli = { workspace = true } codex-utils-oss = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-sandbox-summary = { workspace = true } codex-worktree = { workspace = true } owo-colors = { workspace = true } 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 6aba2618a7..1492502455 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -12,6 +12,7 @@ use codex_core::config::Config; use codex_model_provider_info::WireApi; use codex_protocol::num_format::format_with_separators; use codex_protocol::protocol::SessionConfiguredEvent; +use codex_utils_path_uri::PathUri; use codex_utils_sandbox_summary::summarize_permission_profile; use owo_colors::OwoColorize; use owo_colors::Style; @@ -444,8 +445,12 @@ fn config_summary_entries( "sandbox", summarize_permission_profile( &permission_profile, - &config.cwd, - config.effective_workspace_roots().as_slice(), + &PathUri::from_abs_path(&config.cwd), + &config + .effective_workspace_roots() + .iter() + .map(PathUri::from_abs_path) + .collect::>(), ), ), ]; diff --git a/codex-rs/exec/src/event_processor_with_human_output_tests.rs b/codex-rs/exec/src/event_processor_with_human_output_tests.rs index c91d1388d1..594e2d3bb6 100644 --- a/codex-rs/exec/src/event_processor_with_human_output_tests.rs +++ b/codex-rs/exec/src/event_processor_with_human_output_tests.rs @@ -15,6 +15,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SessionConfiguredEvent; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::test_path_buf; +use codex_utils_path_uri::PathUri; use codex_utils_sandbox_summary::summarize_permission_profile; use owo_colors::Style; use pretty_assertions::assert_eq; @@ -106,7 +107,7 @@ fn reasoning_text_uses_raw_content_when_enabled() { #[test] fn summarizes_disabled_permission_profile_as_danger_full_access() { - let cwd = test_path_buf("/tmp").abs(); + let cwd = PathUri::from_abs_path(&test_path_buf("/tmp").abs()); assert_eq!( summarize_permission_profile( @@ -120,7 +121,7 @@ fn summarizes_disabled_permission_profile_as_danger_full_access() { #[test] fn summarizes_external_permission_profile() { - let cwd = test_path_buf("/tmp").abs(); + let cwd = PathUri::from_abs_path(&test_path_buf("/tmp").abs()); assert_eq!( summarize_permission_profile( @@ -157,14 +158,18 @@ fn summarizes_managed_workspace_write_permission_profile() { ); assert_eq!( - summarize_permission_profile(&profile, &cwd, &[cwd.clone(), cache_root.clone()]), + summarize_permission_profile( + &profile, + &PathUri::from_abs_path(&cwd), + &[cwd.clone().into(), cache_root.clone().into()], + ), format!("workspace-write [workdir, {}]", cache_root.display()) ); } #[test] fn summarizes_managed_read_only_permission_profile() { - let cwd = test_path_buf("/tmp/project").abs(); + let cwd = PathUri::from_abs_path(&test_path_buf("/tmp/project").abs()); let profile = PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::restricted(Vec::new()), NetworkSandboxPolicy::Restricted, diff --git a/codex-rs/tui/src/chatwidget/status_surfaces.rs b/codex-rs/tui/src/chatwidget/status_surfaces.rs index af547ce968..dd45d1b1c4 100644 --- a/codex-rs/tui/src/chatwidget/status_surfaces.rs +++ b/codex-rs/tui/src/chatwidget/status_surfaces.rs @@ -1200,9 +1200,15 @@ fn permissions_display(config: &Config) -> String { } let permission_profile = config.permissions.effective_permission_profile(); - let workspace_roots = config.effective_workspace_roots(); - let summary = - summarize_permission_profile(&permission_profile, &config.cwd, workspace_roots.as_slice()); + let summary = summarize_permission_profile( + &permission_profile, + &PathUri::from_abs_path(&config.cwd), + &config + .effective_workspace_roots() + .iter() + .map(PathUri::from_abs_path) + .collect::>(), + ); 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 86437fe8d2..c2ba3f8c52 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -22,7 +22,7 @@ use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_READ_ONLY; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; use codex_protocol::models::PermissionProfile; use codex_protocol::openai_models::ReasoningEffort; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use codex_utils_sandbox_summary::summarize_permission_profile; use ratatui::prelude::*; use ratatui::style::Stylize; @@ -298,7 +298,12 @@ 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(); + let workspace_roots = config + .effective_workspace_roots() + .iter() + .map(PathUri::from_abs_path) + .collect::>(); + let cwd = PathUri::from_abs_path(&config.cwd); let model_provider = model_provider_id .filter(|id| !id.trim().is_empty()) .map(str::to_string); @@ -311,11 +316,7 @@ impl StatusHistoryCell { ), ( "sandbox", - summarize_permission_profile( - &permission_profile, - &config.cwd, - workspace_roots.as_slice(), - ), + summarize_permission_profile(&permission_profile, &cwd, &workspace_roots), ), ]; if let Some(provider_id) = &model_provider { @@ -342,9 +343,8 @@ impl StatusHistoryCell { .map(|(_, v)| v.clone()) .unwrap_or_else(|| "".to_string()); let active_permission_profile = config.permissions.active_permission_profile(); - let sandbox = - status_permission_summary(&permission_profile, &config.cwd, workspace_roots.as_slice()); - let workspace_root_suffix = workspace_root_suffix(workspace_roots.as_slice(), &config.cwd); + let sandbox = status_permission_summary(&permission_profile, &cwd, &workspace_roots); + let workspace_root_suffix = workspace_root_suffix(&workspace_roots, &cwd); let approval = status_approval_label(approval_policy, config.approvals_reviewer, &approval); let permissions = status_permissions_label( active_permission_profile.as_ref(), @@ -610,8 +610,8 @@ impl StatusHistoryCell { fn status_permission_summary( permission_profile: &PermissionProfile, - cwd: &AbsolutePathBuf, - workspace_roots: &[AbsolutePathBuf], + cwd: &PathUri, + workspace_roots: &[PathUri], ) -> String { let summary = summarize_permission_profile(permission_profile, cwd, workspace_roots); if let Some(details) = summary.strip_prefix("read-only") { @@ -632,14 +632,11 @@ fn status_permission_summary( summary } -fn workspace_root_suffix( - workspace_roots: &[AbsolutePathBuf], - cwd: &AbsolutePathBuf, -) -> Option { +fn workspace_root_suffix(workspace_roots: &[PathUri], cwd: &PathUri) -> Option { let extra_roots = workspace_roots .iter() - .filter(|root| *root != cwd) - .map(|root| root.to_string_lossy().to_string()) + .filter(|root| root.to_string() != cwd.to_string()) + .map(PathUri::inferred_native_path_string) .collect::>(); if extra_roots.is_empty() { None diff --git a/codex-rs/utils/sandbox-summary/Cargo.toml b/codex-rs/utils/sandbox-summary/Cargo.toml index ab6df005da..413b64214b 100644 --- a/codex-rs/utils/sandbox-summary/Cargo.toml +++ b/codex-rs/utils/sandbox-summary/Cargo.toml @@ -9,7 +9,7 @@ workspace = true [dependencies] codex-protocol = { workspace = true } -codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } [dev-dependencies] codex-utils-absolute-path = { workspace = true } diff --git a/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs b/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs index 47e801bec4..6cc420cef6 100644 --- a/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs +++ b/codex-rs/utils/sandbox-summary/src/sandbox_summary.rs @@ -1,7 +1,11 @@ use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxPolicyContext; +use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::protocol::NetworkAccess; use codex_protocol::protocol::SandboxPolicy; -use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathConvention; +use codex_utils_path_uri::PathUri; pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { match sandbox_policy { @@ -53,53 +57,105 @@ pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { pub fn summarize_permission_profile( permission_profile: &PermissionProfile, - cwd: &AbsolutePathBuf, - workspace_roots: &[AbsolutePathBuf], + cwd: &PathUri, + workspace_roots: &[PathUri], ) -> String { - match permission_profile.to_legacy_sandbox_policy(cwd.as_path()) { - Ok(SandboxPolicy::WorkspaceWrite { - network_access, - exclude_tmpdir_env_var, - exclude_slash_tmp, - .. - }) => { - let mut summary = "workspace-write".to_string(); - let mut writable_entries = vec!["workdir".to_string()]; - if !exclude_slash_tmp { - writable_entries.push("/tmp".to_string()); + let network_access = permission_profile.network_sandbox_policy().is_enabled(); + let mut summary = match permission_profile { + PermissionProfile::Disabled => return "danger-full-access".to_string(), + PermissionProfile::External { .. } => "external-sandbox".to_string(), + PermissionProfile::Managed { .. } => { + let policy = permission_profile.file_system_sandbox_policy(); + let context = FileSystemSandboxPolicyContext { + cwd, + workspace_roots, + user_home_dir: None, + temporary_directories: None, + }; + if policy.has_full_disk_write_access_with_context(&context) { + return if network_access { + "danger-full-access".to_string() + } else { + "external-sandbox".to_string() + }; } - if !exclude_tmpdir_env_var { - writable_entries.push("$TMPDIR".to_string()); - } - writable_entries.extend( - workspace_roots - .iter() - .filter(|root| *root != cwd) - .map(|root| root.to_string_lossy().to_string()), - ); - summary.push_str(&format!(" [{}]", writable_entries.join(", "))); - if network_access { - summary.push_str(" (network access enabled)"); + // Keep the legacy summary categories without projecting executor + // paths into the controller's filesystem namespace. + let mut workspace_writable = false; + let mut other_writes = false; + let mut tmpdir_writable = false; + let mut slash_tmp_writable = false; + for entry in policy + .entries + .iter() + .filter(|entry| entry.access.can_write()) + { + match &entry.path { + FileSystemPath::Path { path } => { + if path.to_string() == cwd.to_string() { + workspace_writable = true; + } else { + other_writes = true; + } + } + FileSystemPath::Special { value } => match value { + FileSystemSpecialPath::ProjectRoots { subpath: None } => { + workspace_writable = true; + } + FileSystemSpecialPath::ProjectRoots { subpath: Some(_) } => { + other_writes = true; + } + FileSystemSpecialPath::Root => other_writes = true, + FileSystemSpecialPath::Tmpdir => tmpdir_writable = true, + FileSystemSpecialPath::SlashTmp => slash_tmp_writable = true, + FileSystemSpecialPath::Minimal | FileSystemSpecialPath::Unknown { .. } => {} + }, + FileSystemPath::GlobPattern { .. } => {} + } } - summary - } - Ok(policy) => summarize_sandbox_policy(&policy), - Err(_) => { - if permission_profile.network_sandbox_policy().is_enabled() { - "custom permissions (network access enabled)".to_string() - } else { + + if workspace_writable { + let mut writable_entries = vec!["workdir".to_string()]; + if slash_tmp_writable { + writable_entries.push("/tmp".to_string()); + } + if tmpdir_writable { + writable_entries.push("$TMPDIR".to_string()); + } + writable_entries.extend( + workspace_roots + .iter() + .filter(|root| root.to_string() != cwd.to_string()) + .map(PathUri::inferred_native_path_string), + ); + format!("workspace-write [{}]", writable_entries.join(", ")) + } else if other_writes + || tmpdir_writable + || (cwd.infer_path_convention() == Some(PathConvention::Posix) + && slash_tmp_writable) + { "custom permissions".to_string() + } else { + "read-only".to_string() } } + }; + if network_access { + summary.push_str(" (network access enabled)"); } + summary } #[cfg(test)] mod tests { use super::*; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::LegacyAppPathString; use pretty_assertions::assert_eq; #[test] @@ -168,8 +224,11 @@ mod tests { /*exclude_slash_tmp*/ false, ); - let summary = - summarize_permission_profile(&profile, &cwd, &[cwd.clone(), extra_root.clone()]); + let summary = summarize_permission_profile( + &profile, + &PathUri::from_abs_path(&cwd), + &[cwd.clone().into(), extra_root.clone().into()], + ); assert_eq!( summary, @@ -179,4 +238,82 @@ mod tests { ) ); } + + #[test] + fn permission_profile_summary_preserves_executor_workspace_paths() { + for (cwd, extra_root, expected) in [ + ( + "file:///workspace/repo", + "file:///workspace/shared%20files", + "workspace-write [workdir, /workspace/shared files]", + ), + ( + "file:///C:/workspace/repo", + "file:///C:/workspace/shared%20files", + r"workspace-write [workdir, C:\workspace\shared files]", + ), + ( + "file://server/share/repo", + "file://server/share/shared%20files", + r"workspace-write [workdir, \\server\share\shared files]", + ), + ( + "file:///C:/repo", + "file:///C:/REPO", + r"workspace-write [workdir, C:\REPO]", + ), + ] { + let cwd = PathUri::parse(cwd).expect("executor cwd"); + let extra_root = PathUri::parse(extra_root).expect("executor workspace root"); + let roots = [cwd.clone(), extra_root]; + let profile = PermissionProfile::workspace_write_with_path_uris( + &[], + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ true, + /*exclude_slash_tmp*/ true, + ) + .materialize_project_roots_with_path_uris(&roots); + + assert_eq!( + summarize_permission_profile(&profile, &cwd, &roots), + expected + ); + + let profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + roots[1].clone().into(), + FileSystemAccessMode::Write, + )]), + NetworkSandboxPolicy::Restricted, + ); + assert_eq!( + summarize_permission_profile(&profile, &cwd, &roots), + "custom permissions" + ); + } + } + + #[test] + fn permission_profile_summary_keeps_opaque_workspace_subpath_writes_custom() { + let cwd = LegacyAppPathString::from_string("/C:/repo") + .to_path_uri(PathConvention::Posix) + .expect("opaque POSIX cwd"); + for subpath in ["src", "", "."] { + let profile = PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::ProjectRoots { + subpath: Some(subpath.to_string()), + }, + }, + FileSystemAccessMode::Write, + )]), + NetworkSandboxPolicy::Restricted, + ); + assert_eq!( + summarize_permission_profile(&profile, &cwd, std::slice::from_ref(&cwd)), + "custom permissions" + ); + } + } }