diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index 2d93e07d1e..a3cf0d5717 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs index 9360934aa4..38747f0ada 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/permissions.rs @@ -20,7 +20,7 @@ use codex_protocol::request_permissions::PermissionGrantScope as CorePermissionG use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::LegacyAppPathString; -use codex_utils_path_uri::PathConvention; +use codex_utils_path_uri::PathUri; use serde::Deserialize; use serde::Serialize; use std::io; @@ -69,6 +69,13 @@ pub struct AdditionalFileSystemPermissions { pub entries: Option>, } +fn permission_path_uri(path: LegacyAppPathString) -> io::Result { + if let Ok(path) = AbsolutePathBuf::try_from(path.clone()) { + return Ok(path.into()); + } + PathUri::try_from(path).map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err)) +} + // TODO(anp): Remove this conversion once core permission paths use PathUri. impl From for AdditionalFileSystemPermissions { fn from(value: CoreFileSystemPermissions) -> Self { @@ -109,9 +116,27 @@ impl From for AdditionalFileSystemPermissions { entries: Some(entries), } } else { + let mut read = Vec::new(); + let mut write = Vec::new(); + let legacy_compatible = value.glob_scan_max_depth.is_none() + && value.entries.iter().all(|entry| { + let CoreFileSystemPath::Path { path } = &entry.path else { + return false; + }; + let legacy_path = LegacyAppPathString::from(path.clone()); + if legacy_path.to_inferred_path_uri().as_ref() != Some(path) { + return false; + } + match entry.access { + CoreFileSystemAccessMode::Read => read.push(legacy_path), + CoreFileSystemAccessMode::Write => write.push(legacy_path), + CoreFileSystemAccessMode::Deny => return false, + } + true + }); Self { - read: None, - write: None, + read: (legacy_compatible && !read.is_empty()).then_some(read), + write: (legacy_compatible && !write.is_empty()).then_some(write), glob_scan_max_depth: value.glob_scan_max_depth, entries: Some( value @@ -144,11 +169,7 @@ impl TryFrom for CoreFileSystemPermissions { .map(|paths| { paths .into_iter() - .map(|path| { - path.to_path_uri(PathConvention::native()) - .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))? - .to_abs_path() - }) + .map(permission_path_uri) .collect::>>() }) .transpose()?; @@ -157,15 +178,11 @@ impl TryFrom for CoreFileSystemPermissions { .map(|paths| { paths .into_iter() - .map(|path| { - path.to_path_uri(PathConvention::native()) - .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))? - .to_abs_path() - }) + .map(permission_path_uri) .collect::>>() }) .transpose()?; - CoreFileSystemPermissions::from_read_write_roots(read, write) + CoreFileSystemPermissions::from_read_write_path_uris(read, write) }; permissions.glob_scan_max_depth = value.glob_scan_max_depth; Ok(permissions) @@ -312,9 +329,7 @@ pub enum FileSystemPath { impl From for FileSystemPath { fn from(value: CoreFileSystemPath) -> Self { match value { - CoreFileSystemPath::Path { path } => Self::Path { - path: LegacyAppPathString::from_abs_path(&path), - }, + CoreFileSystemPath::Path { path } => Self::Path { path: path.into() }, CoreFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, CoreFileSystemPath::Special { value } => Self::Special { value: value.into(), @@ -330,10 +345,7 @@ impl TryFrom for CoreFileSystemPath { fn try_from(value: FileSystemPath) -> Result { Ok(match value { FileSystemPath::Path { path } => Self::Path { - path: path - .to_path_uri(PathConvention::native()) - .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))? - .to_abs_path()?, + path: permission_path_uri(path)?, }, FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, FileSystemPath::Special { value } => Self::Special { diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 4046218e34..64f9af707a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -839,6 +839,52 @@ fn additional_file_system_permissions_preserves_canonical_entries() { .expect("API paths should convert to native paths"), core_permissions ); + + for path in [r"C:\workspace\read-only", r"\\server\share\read-only"] { + let path = LegacyAppPathString::from_string(path); + let core_permissions = CoreFileSystemPermissions::from_read_write_path_uris( + Some(vec![ + PathUri::try_from(path.clone()).expect("valid foreign permission path"), + ]), + /*write*/ None, + ); + let permissions = AdditionalFileSystemPermissions::from(core_permissions.clone()); + assert_eq!(permissions.read, Some(vec![path])); + assert_eq!(permissions.entries.as_ref().map(Vec::len), Some(1)); + assert_eq!( + CoreFileSystemPermissions::try_from(permissions) + .expect("foreign API paths should round-trip"), + core_permissions + ); + } + #[cfg(windows)] + for path in ["//server/share/read-only", r"/\server/share/read-only"] { + let path = LegacyAppPathString::from_string(path); + let core_permissions = + CoreFileSystemPermissions::try_from(AdditionalFileSystemPermissions { + read: Some(vec![path.clone()]), + write: None, + glob_scan_max_depth: None, + entries: None, + }) + .expect("native slash UNC permission path"); + let permissions = AdditionalFileSystemPermissions::from(core_permissions); + assert_eq!( + permissions.read, + Some(vec![LegacyAppPathString::from_string( + r"\\server\share\read-only" + )]) + ); + } + assert!( + CoreFileSystemPermissions::try_from(AdditionalFileSystemPermissions { + read: Some(vec![LegacyAppPathString::from_string(r"\\localhost\share")]), + write: None, + glob_scan_max_depth: None, + entries: None, + }) + .is_ok() + ); } #[test] diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 77513d8202..a56213235d 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -2226,14 +2226,14 @@ async fn default_permissions_profile_populates_runtime_sandbox_policy() -> std:: }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: cwd_root.clone(), + path: cwd_root.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: cwd_root.join("docs"), + path: cwd_root.join("docs").into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, @@ -2923,7 +2923,8 @@ async fn explicit_builtin_workspace_profile_ignores_legacy_workspace_write_setti assert!( !policy.entries.iter().any(|entry| matches!( &entry.path, - FileSystemPath::Path { path } if path.as_path() == extra_root.path() + FileSystemPath::Path { path } + if path.to_abs_path().is_ok_and(|path| path.as_path() == extra_root.path()) )), "explicit :workspace should not inherit sandbox_workspace_write roots as concrete grants, \ policy: {policy:?}" @@ -4437,7 +4438,7 @@ exclude_slash_tmp = true file_system_policy .entries .contains(&FileSystemSandboxEntry { - path: FileSystemPath::Path { path: cwd.abs() }, + path: cwd.abs().into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }) @@ -4447,7 +4448,7 @@ exclude_slash_tmp = true .entries .contains(&FileSystemSandboxEntry { path: FileSystemPath::Path { - path: extra_root.clone(), + path: extra_root.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -4462,7 +4463,8 @@ exclude_slash_tmp = true path: AbsolutePathBuf::resolve_path_against_base( subpath, cwd.path() - ), + ) + .into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: Some( @@ -10442,7 +10444,7 @@ async fn permission_profile_override_preserves_split_write_roots() -> std::io::R }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: outside_root.clone(), + path: outside_root.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs index c14d0b7dc3..092bb33a56 100644 --- a/codex-rs/core/src/config/permissions.rs +++ b/codex-rs/core/src/config/permissions.rs @@ -243,7 +243,7 @@ fn insert_filesystem_permission_toml( match entry.path { FileSystemPath::Path { path } => { entries.insert( - path.into_path_buf().to_string_lossy().into_owned(), + path.inferred_native_path_string(), FilesystemPermissionToml::Access(entry.access), ); } diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index eec5fa1b0a..bc28012e99 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -154,7 +154,7 @@ fn render_file_system_entry(rendered: &mut String, entry: &FileSystemSandboxEntr rendered.push_str("\">"); match &entry.path { FileSystemPath::Path { path } => { - push_text_element(rendered, "path", path.to_string_lossy().as_ref()); + push_text_element(rendered, "path", &path.inferred_native_path_string()); } FileSystemPath::GlobPattern { pattern } => { push_text_element(rendered, "glob", pattern); diff --git a/codex-rs/core/src/safety_tests.rs b/codex-rs/core/src/safety_tests.rs index 29afacde0c..cf947d8d2b 100644 --- a/codex-rs/core/src/safety_tests.rs +++ b/codex-rs/core/src/safety_tests.rs @@ -231,7 +231,7 @@ fn explicit_unreadable_paths_prevent_auto_approval_for_external_sandbox() { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: blocked_absolute, + path: blocked_absolute.into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -281,7 +281,7 @@ fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: docs_absolute, + path: docs_absolute.into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, @@ -325,7 +325,7 @@ fn missing_project_dot_codex_config_requires_approval() { .entries .push(FileSystemSandboxEntry { path: FileSystemPath::Path { - path: cwd.join(".codex"), + path: cwd.join(".codex").into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, diff --git a/codex-rs/core/src/sandbox_tags_tests.rs b/codex-rs/core/src/sandbox_tags_tests.rs index e96f3eccc9..b4feee5b2f 100644 --- a/codex-rs/core/src/sandbox_tags_tests.rs +++ b/codex-rs/core/src/sandbox_tags_tests.rs @@ -146,7 +146,7 @@ fn profile_policy_tag_reports_closest_legacy_mode() { glob_scan_max_depth: None, entries: vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: writable_root, + path: writable_root.into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index cdd0f70604..948825aabd 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5008,7 +5008,7 @@ async fn session_configuration_apply_preserves_profile_file_system_policy_on_cwd missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs_dir }, + path: docs_dir.into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, @@ -5115,7 +5115,7 @@ async fn session_configuration_apply_permission_profile_accepts_direct_write_roo let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: external_write_path.clone(), + path: external_write_path.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -5373,7 +5373,7 @@ async fn session_configuration_apply_preserves_absolute_cwd_write_root_on_cwd_up }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: original_cwd.clone(), + path: original_cwd.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -6733,7 +6733,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir file_system: Some(FileSystemPermissions { entries: vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: environment_cwd.join("relative.txt"), + path: environment_cwd.join("relative.txt").into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -9577,7 +9577,11 @@ async fn turn_context_item_stores_split_file_system_sandbox_policy_when_differen assert_eq!( item.file_system_sandbox_policy, - Some(file_system_sandbox_policy) + Some( + file_system_sandbox_policy + .try_into() + .expect("serializable split policy"), + ) ); assert_eq!( item.permission_profile, @@ -9766,7 +9770,11 @@ async fn record_context_updates_and_set_reference_context_item_persists_split_fi }); assert_eq!( persisted_file_system_sandbox_policy, - Some(file_system_sandbox_policy) + Some( + file_system_sandbox_policy + .try_into() + .expect("serializable split policy"), + ) ); } diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index e4592b5dc4..042ebf3d3f 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -16,6 +16,7 @@ use codex_protocol::config_types::ShellEnvironmentPolicy; use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::ModelInfo; +use codex_protocol::permissions::RawFileSystemSandboxPolicy; use codex_protocol::protocol::EnvironmentConfig; use codex_protocol::protocol::EnvironmentConfigState; use codex_protocol::protocol::ErrorEvent; @@ -444,7 +445,7 @@ impl TurnContext { } } - fn non_legacy_file_system_sandbox_policy(&self) -> Option { + fn non_legacy_file_system_sandbox_policy(&self) -> Option { // Omit the derived split filesystem policy when it is equivalent to // the legacy sandbox policy. This keeps turn-context payloads stable // while both fields exist; once callers consume only the split policy, @@ -456,8 +457,11 @@ impl TurnContext { &self.cwd, ); let file_system_sandbox_policy = self.file_system_sandbox_policy(); + // `permission_profile` below is authoritative and serializes the same + // runtime entries, so this compatibility field may omit an unrenderable policy. (file_system_sandbox_policy != legacy_file_system_sandbox_policy) - .then_some(file_system_sandbox_policy) + .then(|| file_system_sandbox_policy.try_into().ok()) + .flatten() } pub(crate) fn to_turn_context_item(&self) -> TurnContextItem { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 67205e0366..8783c21734 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -300,14 +300,18 @@ fn shell_request_escalation_execution_is_explicit() { let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::from_absolute_path("/tmp/original/output").unwrap(), + path: AbsolutePathBuf::from_absolute_path("/tmp/original/output") + .unwrap() + .into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::from_absolute_path("/tmp/secret").unwrap(), + path: AbsolutePathBuf::from_absolute_path("/tmp/secret") + .unwrap() + .into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 79b007dce5..e9098509d6 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -234,7 +234,7 @@ fn windows_sandbox_env_preserves_denied_reads_or_rejects_unsupported_backend() { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: denied_path.clone(), + path: denied_path.clone().into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 63ddaab8bc..17264f4edb 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -60,6 +60,8 @@ use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::user_input::UserInput; use codex_thread_store::ThreadStore; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use serde_json::Value; @@ -124,6 +126,21 @@ pub fn local(cwd: AbsolutePathBuf) -> TurnEnvironmentSelection { } } +/// Converts the host-shaped /C:/... cwd projection used by Wine tests back +/// into the selected executor's Windows URI. +pub fn executor_path_uri(path: impl AsRef) -> Result { + let path = path.as_ref(); + if matches!(test_environment(), TestEnvironment::WineExec) + && let Some(path) = path.to_str() + && matches!(path.as_bytes(), [b'/', drive, b':', b'/' | b'\\', ..] if drive.is_ascii_alphabetic()) + { + return LegacyAppPathString::from_string(path[1..].to_string()) + .to_path_uri(PathConvention::Windows) + .map_err(Into::into); + } + Ok(PathUri::from_host_native_path(path)?) +} + pub fn local_selections(cwd: AbsolutePathBuf) -> TurnEnvironmentSelections { TurnEnvironmentSelections::new(cwd.clone(), vec![local(cwd)]) } @@ -644,7 +661,7 @@ impl TestCodexBuilder { cwd: Arc, home: Arc, resume_from: Option, - test_env: TestEnv, + mut test_env: TestEnv, environment_manager: Arc, ) -> anyhow::Result { let auth = self.auth.clone(); @@ -740,9 +757,23 @@ impl TestCodexBuilder { .await? } (None, None) => { + let environments = if test_env.selection().cwd.infer_path_convention() + == Some(PathConvention::Windows) + && PathUri::from_abs_path(&config.cwd) != test_env.selection().cwd + { + let cwd = executor_path_uri(&config.cwd)?; + let mut selection = test_env.selection().clone(); + selection.cwd = cwd.clone(); + selection.workspace_roots = vec![cwd]; + test_env.selection = selection.clone(); + Some(vec![selection]) + } else { + None + }; Box::pin(thread_manager.start_thread(StartThreadOptions { history_mode: self.history_mode, client_mcp_extensions: client_mcp_extensions(), + environments, ..StartThreadOptions::new(config.clone()) })) .await? @@ -864,6 +895,14 @@ impl TestCodex { self.cwd_path().join(rel) } + pub fn workspace_path_uri(&self, rel: impl AsRef) -> Result { + let rel = rel + .as_ref() + .to_str() + .context("test workspace path must be UTF-8")?; + Ok(self.executor_environment().selection().cwd.join(rel)?) + } + pub fn executor_environment(&self) -> &TestEnv { &self._test_env } @@ -1096,9 +1135,8 @@ impl TestCodexHarness { rel: impl AsRef, contents: impl AsRef<[u8]>, ) -> Result<()> { - let abs_path = self.path_abs(rel); - if let Some(parent) = abs_path.parent() { - let parent_uri = PathUri::from_host_native_path(&parent)?; + let path_uri = self.test.workspace_path_uri(rel)?; + if let Some(parent_uri) = path_uri.parent() { self.test .fs() .create_directory( @@ -1108,21 +1146,15 @@ impl TestCodexHarness { ) .await?; } - let abs_path_uri = PathUri::from_host_native_path(&abs_path)?; self.test .fs() - .write_file( - &abs_path_uri, - contents.as_ref().to_vec(), - /*sandbox*/ None, - ) + .write_file(&path_uri, contents.as_ref().to_vec(), /*sandbox*/ None) .await?; Ok(()) } pub async fn read_file_text(&self, rel: impl AsRef) -> Result { - let path = self.path_abs(rel); - let path_uri = PathUri::from_host_native_path(&path)?; + let path_uri = self.test.workspace_path_uri(rel)?; Ok(self .test .fs() @@ -1131,8 +1163,7 @@ impl TestCodexHarness { } pub async fn create_dir_all(&self, rel: impl AsRef) -> Result<()> { - let path = self.path_abs(rel); - let path_uri = PathUri::from_host_native_path(&path)?; + let path_uri = self.test.workspace_path_uri(rel)?; self.test .fs() .create_directory( @@ -1145,7 +1176,8 @@ impl TestCodexHarness { } pub async fn path_exists(&self, rel: impl AsRef) -> Result { - self.abs_path_exists(&self.path_abs(rel)).await + self.path_uri_exists(&self.test.workspace_path_uri(rel)?) + .await } pub async fn remove_abs_path(&self, path: &AbsolutePathBuf) -> Result<()> { @@ -1166,10 +1198,14 @@ impl TestCodexHarness { pub async fn abs_path_exists(&self, path: &AbsolutePathBuf) -> Result { let path_uri = PathUri::from_abs_path(path); + self.path_uri_exists(&path_uri).await + } + + async fn path_uri_exists(&self, path_uri: &PathUri) -> Result { match self .test .fs() - .get_metadata(&path_uri, /*sandbox*/ None) + .get_metadata(path_uri, /*sandbox*/ None) .await { Ok(_) => Ok(true), diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index 53385ecba4..692fe8b57c 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -30,6 +30,7 @@ use core_test_support::skip_if_no_network; use core_test_support::skip_if_no_remote_env; use core_test_support::test_codex::RecordingUserInstructionsProvider; use core_test_support::test_codex::TestCodexBuilder; +use core_test_support::test_codex::executor_path_uri; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use pretty_assertions::assert_eq; @@ -116,8 +117,8 @@ fn instruction_fragments(request: &responses::ResponsesRequest) -> Vec { .collect() } -fn expected_instruction_fragment(cwd: &AbsolutePathBuf, contents: &str) -> String { - let cwd = PathUri::from_abs_path(cwd).inferred_native_path_string(); +fn expected_instruction_fragment(cwd: &PathUri, contents: &str) -> String { + let cwd = cwd.inferred_native_path_string(); format!("# AGENTS.md instructions for {cwd}\n\n\n{contents}\n") } @@ -185,8 +186,8 @@ async fn agents_override_is_preferred_over_agents_md() -> Result<()> { agents_instructions(test_codex().with_workspace_setup(|cwd, fs| async move { let agents_md = cwd.join("AGENTS.md"); let override_md = cwd.join("AGENTS.override.md"); - let agents_md_uri = PathUri::from_host_native_path(&agents_md)?; - let override_md_uri = PathUri::from_host_native_path(&override_md)?; + let agents_md_uri = executor_path_uri(&agents_md)?; + let override_md_uri = executor_path_uri(&override_md)?; fs.write_file(&agents_md_uri, b"base doc".to_vec(), /*sandbox*/ None) .await?; fs.write_file( @@ -221,8 +222,8 @@ async fn configured_fallback_is_used_when_agents_candidate_is_directory() -> Res .with_workspace_setup(|cwd, fs| async move { let agents_dir = cwd.join("AGENTS.md"); let fallback = cwd.join("WORKFLOW.md"); - let agents_dir_uri = PathUri::from_host_native_path(&agents_dir)?; - let fallback_uri = PathUri::from_host_native_path(&fallback)?; + let agents_dir_uri = executor_path_uri(&agents_dir)?; + let fallback_uri = executor_path_uri(&fallback)?; fs.create_directory( &agents_dir_uri, CreateDirectoryOptions { recursive: true }, @@ -264,10 +265,10 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { let root_agents = root.join("AGENTS.md"); let git_marker = root.join(".git"); let nested_agents = nested.join("AGENTS.md"); - let nested_uri = PathUri::from_host_native_path(&nested)?; - let root_agents_uri = PathUri::from_host_native_path(&root_agents)?; - let git_marker_uri = PathUri::from_host_native_path(&git_marker)?; - let nested_agents_uri = PathUri::from_host_native_path(&nested_agents)?; + let nested_uri = executor_path_uri(&nested)?; + let root_agents_uri = executor_path_uri(&root_agents)?; + let git_marker_uri = executor_path_uri(&git_marker)?; + let nested_agents_uri = executor_path_uri(&nested_agents)?; fs.create_directory( &nested_uri, @@ -407,7 +408,7 @@ async fn selected_environment_sources_match_model_visible_instructions() -> Resu let mut builder = test_codex() .with_home(home) .with_workspace_setup(|cwd, fs| async move { - let agents_md_uri = PathUri::from_host_native_path(cwd.join("AGENTS.md"))?; + let agents_md_uri = executor_path_uri(cwd.join("AGENTS.md"))?; fs.write_file( &agents_md_uri, b"project doc".to_vec(), @@ -417,14 +418,13 @@ async fn selected_environment_sources_match_model_visible_instructions() -> Resu Ok::<(), anyhow::Error>(()) }); let test = builder.build_with_auto_env(&server).await?; - let project_agents = test.config.cwd.join("AGENTS.md"); let global_agents = global_agents.abs(); assert_eq!( test.codex.instruction_sources().await, vec![ PathUri::from_abs_path(&global_agents), - PathUri::from_abs_path(&project_agents), + test.workspace_path_uri("AGENTS.md")?, ] ); @@ -464,8 +464,7 @@ async fn loads_user_instructions_without_a_primary_environment() -> Result<()> { .with_home(Arc::clone(&home)) .with_user_instructions_provider(provider.clone()) .with_workspace_setup(|cwd, fs| async move { - let project_agents_uri = - PathUri::from_host_native_path(cwd.join(GLOBAL_AGENTS_FILENAME))?; + let project_agents_uri = executor_path_uri(cwd.join(GLOBAL_AGENTS_FILENAME))?; fs.write_file( &project_agents_uri, PROJECT_INSTRUCTIONS.as_bytes().to_vec(), @@ -535,7 +534,7 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re let mut builder = test_codex() .with_home(Arc::clone(&home)) .with_workspace_setup(|cwd, fs| async move { - let agents_md_uri = PathUri::from_host_native_path(cwd.join("AGENTS.md"))?; + let agents_md_uri = executor_path_uri(cwd.join("AGENTS.md"))?; fs.write_file( &agents_md_uri, PROJECT_INSTRUCTIONS.as_bytes().to_vec(), @@ -545,10 +544,9 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re Ok(()) }); let test = builder.build_with_auto_env(&server).await?; - let project_source = test.config.cwd.join(GLOBAL_AGENTS_FILENAME); let creation_sources = vec![ PathUri::from_abs_path(&global_source), - PathUri::from_abs_path(&project_source), + test.workspace_path_uri(GLOBAL_AGENTS_FILENAME)?, ]; // Confirm the thread records both creation-time sources in composition order. @@ -564,7 +562,7 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re )?; test.fs() .write_file( - &PathUri::from_host_native_path(&project_source)?, + &test.workspace_path_uri(GLOBAL_AGENTS_FILENAME)?, NEW_PROJECT_INSTRUCTIONS.as_bytes().to_vec(), /*sandbox*/ None, ) @@ -581,7 +579,10 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re assert_eq!(requests.len(), 2); let expected_contents = format!("{GLOBAL_INSTRUCTIONS}\n\n{PROJECT_SEPARATOR}\n\n{PROJECT_INSTRUCTIONS}"); - let expected_fragment = expected_instruction_fragment(&test.config.cwd, &expected_contents); + let expected_fragment = expected_instruction_fragment( + &test.executor_environment().selection().cwd, + &expected_contents, + ); let fragments = instruction_fragments(&requests[0]); assert_eq!(fragments, vec![expected_fragment.clone()]); assert_single_instruction_fragment(&requests[1], &expected_fragment); @@ -643,7 +644,7 @@ async fn multi_environment_project_instructions_share_one_byte_budget() -> Resul .with_config(|config| config.project_doc_max_bytes = 7) .with_workspace_setup(|cwd, fs| async move { fs.write_file( - &PathUri::from_host_native_path(cwd.join(GLOBAL_AGENTS_FILENAME))?, + &executor_path_uri(cwd.join(GLOBAL_AGENTS_FILENAME))?, b"ABCDE".to_vec(), /*sandbox*/ None, ) @@ -657,8 +658,8 @@ async fn multi_environment_project_instructions_share_one_byte_budget() -> Resul environments: Some(vec![ TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: PathUri::from_abs_path(&test.config.cwd), - workspace_roots: vec![PathUri::from_abs_path(&test.config.cwd)], + cwd: test.executor_environment().selection().cwd.clone(), + workspace_roots: vec![test.executor_environment().selection().cwd.clone()], config: EnvironmentConfigState::FromThread, }, TurnEnvironmentSelection { @@ -676,7 +677,10 @@ async fn multi_environment_project_instructions_share_one_byte_budget() -> Resul let contents = format!( "for `{REMOTE_ENVIRONMENT_ID}` with root {}\n\nABCDE\n\nfor `{LOCAL_ENVIRONMENT_ID}` with root {}\n\nVW", - PathUri::from_abs_path(&test.config.cwd).inferred_native_path_string(), + test.executor_environment() + .selection() + .cwd + .inferred_native_path_string(), local_root.path().display(), ); let expected = @@ -722,7 +726,7 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho .with_user_instructions_provider(provider.clone()) .with_workspace_setup(|cwd, fs| async move { fs.write_file( - &PathUri::from_host_native_path(cwd.join(GLOBAL_AGENTS_FILENAME))?, + &executor_path_uri(cwd.join(GLOBAL_AGENTS_FILENAME))?, b"remote project instructions".to_vec(), /*sandbox*/ None, ) @@ -737,8 +741,8 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho environments: Some(vec![ TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: PathUri::from_abs_path(&test.config.cwd), - workspace_roots: vec![PathUri::from_abs_path(&test.config.cwd)], + cwd: test.executor_environment().selection().cwd.clone(), + workspace_roots: vec![test.executor_environment().selection().cwd.clone()], config: EnvironmentConfigState::FromThread, }, TurnEnvironmentSelection { @@ -756,7 +760,7 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho thread.thread.instruction_sources().await, vec![ PathUri::from_abs_path(&global_source), - PathUri::from_abs_path(&remote_source), + executor_path_uri(&remote_source)?, PathUri::from_host_native_path(&local_source)?, ] ); @@ -770,7 +774,7 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho )?; test.fs() .write_file( - &PathUri::from_host_native_path(test.config.cwd.join(GLOBAL_AGENTS_OVERRIDE_FILENAME))?, + &executor_path_uri(test.config.cwd.join(GLOBAL_AGENTS_OVERRIDE_FILENAME))?, b"new remote project instructions".to_vec(), /*sandbox*/ None, ) @@ -783,7 +787,10 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho let contents = format!( "{GLOBAL_INSTRUCTIONS}\n\nfor `{REMOTE_ENVIRONMENT_ID}` with root {}\n\nremote project instructions\n\nfor `{LOCAL_ENVIRONMENT_ID}` with root {}\n\nlocal project instructions", - PathUri::from_abs_path(&test.config.cwd).inferred_native_path_string(), + test.executor_environment() + .selection() + .cwd + .inferred_native_path_string(), local_root.path().display(), ); let expected = @@ -797,7 +804,7 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho thread.thread.instruction_sources().await, vec![ PathUri::from_abs_path(&global_source), - PathUri::from_abs_path(&remote_source), + executor_path_uri(&remote_source)?, PathUri::from_host_native_path(&local_source)?, ] ); diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 2041d3ccb7..3a88ab6d7a 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -59,6 +59,7 @@ use core_test_support::skip_if_target_windows; use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::TestCodexHarness; +use core_test_support::test_codex::executor_path_uri; use core_test_support::test_codex::local; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; @@ -145,7 +146,7 @@ fn workspace_write_with_read_only_root(read_only_root: AbsolutePathBuf) -> Permi let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { - path: read_only_root, + path: read_only_root.into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, @@ -169,7 +170,7 @@ fn workspace_write_with_unreadable_path(unreadable_path: AbsolutePathBuf) -> Per let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { - path: unreadable_path, + path: unreadable_path.into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -2102,7 +2103,7 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()> let harness = apply_patch_harness_with(|builder| { builder.with_workspace_setup(|cwd, fs| async move { - let binary_path_uri = PathUri::from_host_native_path(cwd.join("binary.dat"))?; + let binary_path_uri = executor_path_uri(cwd.join("binary.dat"))?; fs.write_file( &binary_path_uri, vec![0xff, 0xfe, 0xfd], diff --git a/codex-rs/core/tests/suite/extension_sandbox.rs b/codex-rs/core/tests/suite/extension_sandbox.rs index f0d6fa46de..b6d99659c1 100644 --- a/codex-rs/core/tests/suite/extension_sandbox.rs +++ b/codex-rs/core/tests/suite/extension_sandbox.rs @@ -117,7 +117,7 @@ async fn extension_tool_receives_turn_environment_sandbox() -> Result<()> { .entries .push(FileSystemSandboxEntry { path: FileSystemPath::Path { - path: denied_path.clone(), + path: denied_path.clone().into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -202,7 +202,9 @@ async fn extension_tool_uses_granted_turn_permissions_without_host_local_persist std::fs::write(&image_path, TINY_PNG_BYTES)?; let requested_permissions = RequestPermissionProfile { file_system: Some(FileSystemPermissions::from_read_write_roots( - Some(vec![image_dir.path().canonicalize()?.try_into()?]), + Some(vec![AbsolutePathBuf::try_from( + image_dir.path().canonicalize()?, + )?]), Some(Vec::new()), )), ..RequestPermissionProfile::default() diff --git a/codex-rs/core/tests/suite/guardian_review.rs b/codex-rs/core/tests/suite/guardian_review.rs index d1c46a372b..80ec0e88d5 100644 --- a/codex-rs/core/tests/suite/guardian_review.rs +++ b/codex-rs/core/tests/suite/guardian_review.rs @@ -29,7 +29,6 @@ use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::permissions::FileSystemAccessMode; -use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; @@ -477,7 +476,7 @@ async fn guardian_session_is_reused_for_consecutive_tool_reviews_without_prewarm /*exclude_slash_tmp*/ true, ); file_system_policy.entries.push(FileSystemSandboxEntry::new( - FileSystemPath::Path { path: secret_file }, + secret_file.into(), FileSystemAccessMode::Deny, )); config diff --git a/codex-rs/core/tests/suite/openai_file_mcp.rs b/codex-rs/core/tests/suite/openai_file_mcp.rs index 13bcc24bfc..724b45e234 100644 --- a/codex-rs/core/tests/suite/openai_file_mcp.rs +++ b/codex-rs/core/tests/suite/openai_file_mcp.rs @@ -8,7 +8,6 @@ use anyhow::Result; use codex_core::config::Config; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; -use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; @@ -38,6 +37,7 @@ use core_test_support::responses::start_mock_server; use core_test_support::skip_if_sandbox; use core_test_support::skip_if_target_windows; use core_test_support::test_codex::TestCodex; +use core_test_support::test_codex::executor_path_uri; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -58,7 +58,7 @@ fn restrict_apps_upload_reads(config: &mut Config, denied_file_name: &str) { .expect("denied file path should be absolute"); let mut file_system_policy = FileSystemSandboxPolicy::read_only(); file_system_policy.entries.push(FileSystemSandboxEntry::new( - FileSystemPath::Path { path: denied_path }, + denied_path.into(), FileSystemAccessMode::Deny, )); config @@ -232,7 +232,7 @@ async fn codex_apps_file_params_omit_fields_absent_from_tool_schema() -> Result< let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url.clone()) .with_workspace_setup(|cwd, fs| async move { - let report_path = PathUri::from_abs_path(&cwd.join("report.txt")); + let report_path = executor_path_uri(cwd.join("report.txt"))?; fs.write_file( &report_path, vec![b'x'; STREAMED_FILE_SIZE], diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 2cf7dbaf5f..ef55a4868b 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -417,8 +417,8 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> { "run the remote shell in the remote cwd", Some(vec![TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: PathUri::from_abs_path(&test.config.cwd), - workspace_roots: vec![PathUri::from_abs_path(&test.config.cwd)], + cwd: test.executor_environment().selection().cwd.clone(), + workspace_roots: vec![test.executor_environment().selection().cwd.clone()], config: EnvironmentConfigState::FromThread, }]), ) @@ -2483,7 +2483,7 @@ fn read_only_sandbox(readable_root: PathBuf) -> FileSystemSandboxContext { FileSystemSandboxContext::from_permission_profile(PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: readable_root, + path: readable_root.into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, @@ -2497,7 +2497,7 @@ fn workspace_write_sandbox(writable_root: PathBuf) -> FileSystemSandboxContext { FileSystemSandboxContext::from_permission_profile(PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: writable_root, + path: writable_root.into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 89b8936a8a..a8eb8a3d17 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -161,9 +161,8 @@ fn png_bytes(width: u32, height: u32, rgba: [u8; 4]) -> anyhow::Result> Ok(cursor.into_inner()) } -async fn create_workspace_directory(test: &TestCodex, rel_path: &str) -> anyhow::Result { - let abs_path = test.config.cwd.join(rel_path); - let abs_path_uri = PathUri::from_host_native_path(&abs_path)?; +async fn create_workspace_directory(test: &TestCodex, rel_path: &str) -> anyhow::Result { + let abs_path_uri = test.workspace_path_uri(rel_path)?; test.fs() .create_directory( &abs_path_uri, @@ -171,7 +170,7 @@ async fn create_workspace_directory(test: &TestCodex, rel_path: &str) -> anyhow: /*sandbox*/ None, ) .await?; - Ok(abs_path.into_path_buf()) + Ok(abs_path_uri) } async fn write_workspace_file( @@ -179,9 +178,8 @@ async fn write_workspace_file( rel_path: &str, contents: Vec, ) -> anyhow::Result { - let abs_path = test.config.cwd.join(rel_path); - if let Some(parent) = abs_path.parent() { - let parent_uri = PathUri::from_host_native_path(&parent)?; + let abs_path_uri = test.workspace_path_uri(rel_path)?; + if let Some(parent_uri) = abs_path_uri.parent() { test.fs() .create_directory( &parent_uri, @@ -190,11 +188,10 @@ async fn write_workspace_file( ) .await?; } - let abs_path_uri = PathUri::from_host_native_path(&abs_path)?; test.fs() .write_file(&abs_path_uri, contents, /*sandbox*/ None) .await?; - Ok(abs_path.into_path_buf()) + Ok(abs_path_uri.to_path_buf()) } async fn write_workspace_png( @@ -424,14 +421,10 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { let TestCodex { codex, session_configured, - config, .. } = &test; - let cwd = config.cwd.clone(); - let rel_path = "assets/example.png"; - let abs_path = cwd.join(rel_path); - let path_uri = PathUri::from_abs_path(&abs_path); + let path_uri = test.workspace_path_uri(rel_path)?; let original_width = 2304; let original_height = 864; write_workspace_png( @@ -690,7 +683,7 @@ async fn view_image_tool_applies_local_sandbox_read_denies() -> anyhow::Result<( .entries .push(FileSystemSandboxEntry { path: FileSystemPath::Path { - path: denied_path.clone(), + path: denied_path.clone().into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -738,7 +731,7 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<() let local_cwd = TempDir::new()?; fs::write(local_cwd.path().join("remote.png"), b"not a remote image")?; let local_selection = local(local_cwd.path().abs()); - let remote_cwd_uri = PathUri::from_abs_path(test.executor_environment().cwd()); + let remote_cwd_uri = test.executor_environment().selection().cwd.clone(); let image_path_uri = remote_cwd_uri.join("remote.png")?; let png = png_bytes(/*width*/ 1, /*height*/ 1, [0, 255, 0, 255])?; test.fs() @@ -1430,7 +1423,7 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> { .function_call_output_content_and_success(call_id) .and_then(|(content, _)| content) .expect("output text present"); - let expected_path = PathUri::from_host_native_path(&abs_path)?.inferred_native_path_string(); + let expected_path = abs_path.inferred_native_path_string(); let expected_message = format!("image path `{expected_path}` is not a file"); assert_eq!(output_text, expected_message); @@ -1528,11 +1521,8 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> { } = &test; let rel_path = "missing/example.png"; - // Under wine-exec, the executor cwd is stored as a host-compatible `/C:/...` - // projection. Reconstruct its `PathUri` so the expected error uses the selected - // environment's native Windows spelling, matching the handler. - let expected_path = PathUri::from_abs_path(test.executor_environment().cwd()) - .join(rel_path)? + let expected_path = test + .workspace_path_uri(rel_path)? .inferred_native_path_string(); let call_id = "view-image-missing"; diff --git a/codex-rs/core/tests/suite/windows_sandbox.rs b/codex-rs/core/tests/suite/windows_sandbox.rs index b0b5d0375f..a9afe3ae8f 100644 --- a/codex-rs/core/tests/suite/windows_sandbox.rs +++ b/codex-rs/core/tests/suite/windows_sandbox.rs @@ -162,7 +162,7 @@ async fn windows_restricted_token_rejects_exact_and_glob_deny_read_policy() -> a }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: future_secret, + path: future_secret.into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -303,7 +303,7 @@ async fn windows_elevated_enforces_deny_read_and_protects_setup_marker() -> anyh missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: exact_secret }, + path: exact_secret.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -438,7 +438,7 @@ async fn windows_elevated_shell_and_unified_exec_enforce_managed_deny_reads() -> }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: config.cwd.join("exact-secret.txt"), + path: config.cwd.join("exact-secret.txt").into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index 8119a9f1cb..34cb686fa6 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -1058,15 +1058,14 @@ mod tests { let file_system = ManagedFileSystemPermissions::Restricted { entries: vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: native_cwd.clone().try_into().expect("absolute cwd"), - }, + path: FileSystemPath::Path { path: cwd.clone() }, access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry::skip_missing_path( FileSystemPath::Path { - path: native_cwd.join(".git").try_into().expect("absolute path"), + path: PathUri::from_host_native_path(native_cwd.join(".git")) + .expect("absolute path"), }, FileSystemAccessMode::Read, ), @@ -1136,9 +1135,7 @@ mod tests { let cwd = PathUri::from_host_native_path(&native_cwd).expect("cwd URI"); let mut file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::Path { - path: native_cwd.try_into().expect("absolute cwd"), - }, + path: FileSystemPath::Path { path: cwd.clone() }, access: FileSystemAccessMode::Read, missing_path_behavior: None, }]); diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 581d3564a9..ca715ef44e 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -236,8 +236,12 @@ fn add_helper_runtime_permissions( fn normalize_file_system_policy_root_aliases(file_system_policy: &mut FileSystemSandboxPolicy) { for entry in &mut file_system_policy.entries { - if let FileSystemPath::Path { path } = &mut entry.path { - *path = normalize_top_level_alias(path.clone()); + // Alias normalization uses this executor's filesystem; leave foreign + // or opaque PathUris unchanged. + if let FileSystemPath::Path { path } = &mut entry.path + && let Ok(native_path) = path.to_abs_path() + { + *path = normalize_top_level_alias(native_path).into(); } } } diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 29038c763d..40bf0d1b89 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -452,7 +452,7 @@ mod tests { fn remote_sandbox_context_drops_unused_cwd() { let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: absolute_test_path("remote-root"), + path: absolute_test_path("remote-root").into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, diff --git a/codex-rs/exec-server/tests/capability_discovery.rs b/codex-rs/exec-server/tests/capability_discovery.rs index 840a4727d4..c970c91b78 100644 --- a/codex-rs/exec-server/tests/capability_discovery.rs +++ b/codex-rs/exec-server/tests/capability_discovery.rs @@ -15,8 +15,6 @@ use codex_protocol::models::PermissionProfile; #[cfg(unix)] use codex_protocol::permissions::FileSystemAccessMode; #[cfg(unix)] -use codex_protocol::permissions::FileSystemPath; -#[cfg(unix)] use codex_protocol::permissions::FileSystemSandboxEntry; #[cfg(unix)] use codex_protocol::permissions::FileSystemSandboxPolicy; @@ -214,7 +212,7 @@ async fn sandboxed_discovery_follows_only_permitted_external_symlinks() -> anyho let root_path = AbsolutePathBuf::from_absolute_path(root.path())?; let external_root = AbsolutePathBuf::from_absolute_path(external.path())?; let path_entry = - |path, access| FileSystemSandboxEntry::new(FileSystemPath::Path { path }, access); + |path: AbsolutePathBuf, access| FileSystemSandboxEntry::new(path.into(), access); let read_root = path_entry(root_path, FileSystemAccessMode::Read); let read_external = path_entry(external_root.clone(), FileSystemAccessMode::Read); let deny_external_skill = path_entry(external_root.join("skill"), FileSystemAccessMode::Deny); diff --git a/codex-rs/exec-server/tests/file_stream.rs b/codex-rs/exec-server/tests/file_stream.rs index b55c17edcc..cb03cc38b5 100644 --- a/codex-rs/exec-server/tests/file_stream.rs +++ b/codex-rs/exec-server/tests/file_stream.rs @@ -364,7 +364,6 @@ fn read_only_sandbox(path: std::path::PathBuf) -> codex_exec_server::FileSystemS use codex_exec_server::FileSystemSandboxContext; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; - use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; @@ -374,7 +373,7 @@ fn read_only_sandbox(path: std::path::PathBuf) -> codex_exec_server::FileSystemS .unwrap_or_else(|err| panic!("sandbox path should be absolute: {err}")); FileSystemSandboxContext::from_permission_profile(PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::Path { path }, + path: path.into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }]), diff --git a/codex-rs/exec-server/tests/file_system/support.rs b/codex-rs/exec-server/tests/file_system/support.rs index b5f13f549c..682b526b19 100644 --- a/codex-rs/exec-server/tests/file_system/support.rs +++ b/codex-rs/exec-server/tests/file_system/support.rs @@ -85,7 +85,7 @@ pub(crate) fn read_only_sandbox(readable_root: std::path::PathBuf) -> FileSystem let readable_root = absolute_path(readable_root); sandbox_context(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: readable_root, + path: readable_root.into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, @@ -98,7 +98,7 @@ pub(crate) fn workspace_write_sandbox( let writable_root = absolute_path(writable_root); sandbox_context(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: writable_root, + path: writable_root.into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, 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 bb64a172e8..fb03358759 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 @@ -141,13 +141,13 @@ fn summarizes_managed_workspace_write_permission_profile() { let profile = PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { path: cwd.clone() }, + path: cwd.clone().into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: cache_root.clone(), + path: cache_root.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index fa783074fc..12ab7a7f88 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -135,9 +135,7 @@ pub enum ExecFileSystemPath { impl From for ExecFileSystemPath { fn from(value: FileSystemPath) -> Self { match value { - FileSystemPath::Path { path } => Self::Path { - path: PathUri::from_abs_path(&path), - }, + FileSystemPath::Path { path } => Self::Path { path }, FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, FileSystemPath::Special { value } => Self::Special { value }, } @@ -149,9 +147,10 @@ impl TryFrom for FileSystemPath { fn try_from(value: ExecFileSystemPath) -> Result { Ok(match value { - ExecFileSystemPath::Path { path } => Self::Path { - path: path.to_abs_path()?, - }, + ExecFileSystemPath::Path { path } => { + path.to_abs_path()?; + Self::Path { path } + } ExecFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, ExecFileSystemPath::Special { value } => Self::Special { value }, }) diff --git a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs index dbf6da6e2b..4f9142bccc 100644 --- a/codex-rs/linux-sandbox/src/linux_run_main_tests.rs +++ b/codex-rs/linux-sandbox/src/linux_run_main_tests.rs @@ -224,7 +224,7 @@ fn split_only_filesystem_policy_requires_direct_runtime_enforcement() { missing_path_behavior: None, }, codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Path { path: docs }, + path: docs.into(), access: codex_protocol::permissions::FileSystemAccessMode::Read, missing_path_behavior: None, }, @@ -250,7 +250,7 @@ fn root_write_read_only_carveout_requires_direct_runtime_enforcement() { missing_path_behavior: None, }, codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Path { path: docs }, + path: docs.into(), access: codex_protocol::permissions::FileSystemAccessMode::Read, missing_path_behavior: None, }, @@ -573,7 +573,7 @@ fn resolve_permission_profile_preserves_direct_runtime_profile() { missing_path_behavior: None, }, codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Path { path: docs }, + path: docs.into(), access: codex_protocol::permissions::FileSystemAccessMode::Write, missing_path_behavior: None, }, @@ -629,7 +629,7 @@ fn legacy_landlock_rejects_split_only_filesystem_policies() { missing_path_behavior: None, }, codex_protocol::permissions::FileSystemSandboxEntry { - path: codex_protocol::permissions::FileSystemPath::Path { path: docs }, + path: docs.into(), access: codex_protocol::permissions::FileSystemAccessMode::Write, missing_path_behavior: None, }, diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 5d4ed8bd01..4acbc30351 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -805,21 +805,26 @@ async fn sandbox_blocks_explicit_split_policy_carveouts_under_bwrap() { FileSystemSandboxEntry { path: FileSystemPath::Path { path: AbsolutePathBuf::try_from(sandbox_helper_dir.as_path()) - .expect("absolute helper dir"), + .expect("absolute helper dir") + .into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir"), + path: AbsolutePathBuf::try_from(tmpdir.path()) + .expect("absolute tempdir") + .into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"), + path: AbsolutePathBuf::try_from(blocked.as_path()) + .expect("absolute blocked dir") + .into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -877,28 +882,35 @@ async fn sandbox_reenables_writable_subpaths_under_unreadable_parents() { FileSystemSandboxEntry { path: FileSystemPath::Path { path: AbsolutePathBuf::try_from(sandbox_helper_dir.as_path()) - .expect("absolute helper dir"), + .expect("absolute helper dir") + .into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(tmpdir.path()).expect("absolute tempdir"), + path: AbsolutePathBuf::try_from(tmpdir.path()) + .expect("absolute tempdir") + .into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"), + path: AbsolutePathBuf::try_from(blocked.as_path()) + .expect("absolute blocked dir") + .into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(allowed.as_path()).expect("absolute allowed dir"), + path: AbsolutePathBuf::try_from(allowed.as_path()) + .expect("absolute allowed dir") + .into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -952,7 +964,9 @@ async fn sandbox_blocks_root_read_carveouts_under_bwrap() { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(blocked.as_path()).expect("absolute blocked dir"), + path: AbsolutePathBuf::try_from(blocked.as_path()) + .expect("absolute blocked dir") + .into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/prompts/src/permissions_instructions_tests.rs b/codex-rs/prompts/src/permissions_instructions_tests.rs index b558f2a983..bf1bf07d4d 100644 --- a/codex-rs/prompts/src/permissions_instructions_tests.rs +++ b/codex-rs/prompts/src/permissions_instructions_tests.rs @@ -191,7 +191,7 @@ fn builds_permissions_from_profile() { let permission_profile = PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: writable_root.clone(), + path: writable_root.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -235,7 +235,7 @@ fn builds_permissions_from_profile_with_denied_reads() { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: denied_root.clone(), + path: denied_root.clone().into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index dad99f2dc4..ba23dc2fa2 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -20,6 +20,7 @@ use crate::permissions::FileSystemSandboxKind; use crate::permissions::FileSystemSandboxPolicy; use crate::permissions::FileSystemSpecialPath; use crate::permissions::NetworkSandboxPolicy; +use crate::permissions::RawFileSystemSandboxEntry; use crate::protocol::SandboxPolicy; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -28,6 +29,7 @@ use schemars::JsonSchema; use crate::ResponseItemId; use crate::mcp::CallToolResult; +use codex_utils_path_uri::PathUri; mod executed_tool_calls; @@ -78,6 +80,8 @@ impl SandboxPermissions { #[derive(Debug, Clone, Default, Eq, Hash, PartialEq, JsonSchema, TS)] pub struct FileSystemPermissions { + #[schemars(with = "Vec")] + #[ts(as = "Vec")] pub entries: Vec, pub glob_scan_max_depth: Option, } @@ -100,6 +104,20 @@ impl FileSystemPermissions { read: Option>, write: Option>, ) -> Self { + Self::from_paths(read, write) + } + + pub fn from_read_write_path_uris( + read: Option>, + write: Option>, + ) -> Self { + Self::from_paths(read, write) + } + + fn from_paths

(read: Option>, write: Option>) -> Self + where + P: Into, + { let mut entries = Vec::new(); if let Some(read) = read { entries.extend( @@ -137,9 +155,10 @@ impl FileSystemPermissions { let FileSystemPath::Path { path } = &entry.path else { return None; }; + let path = path.to_abs_path().ok()?; match entry.access { - FileSystemAccessMode::Read => read.push(path.clone()), - FileSystemAccessMode::Write => write.push(path.clone()), + FileSystemAccessMode::Read => read.push(path), + FileSystemAccessMode::Write => write.push(path), FileSystemAccessMode::Deny => return None, } } @@ -155,7 +174,7 @@ impl FileSystemPermissions { #[serde(deny_unknown_fields)] struct CanonicalFileSystemPermissions { #[serde(default, skip_serializing_if = "Vec::is_empty")] - entries: Vec, + entries: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] glob_scan_max_depth: Option, } @@ -176,7 +195,13 @@ impl Serialize for FileSystemPermissions { legacy.serialize(serializer) } else { CanonicalFileSystemPermissions { - entries: self.entries.clone(), + entries: self + .entries + .clone() + .into_iter() + .map(TryInto::try_into) + .collect::>() + .map_err(serde::ser::Error::custom)?, glob_scan_max_depth: self.glob_scan_max_depth, } .serialize(serializer) @@ -194,7 +219,11 @@ impl<'de> Deserialize<'de> for FileSystemPermissions { entries, glob_scan_max_depth, }) => Ok(Self { - entries, + entries: entries + .into_iter() + .map(TryInto::try_into) + .collect::>() + .map_err(serde::de::Error::custom)?, glob_scan_max_depth, }), FileSystemPermissionsDe::Legacy(LegacyReadWriteRoots { read, write }) => { @@ -254,7 +283,7 @@ impl SandboxEnforcement { } /// Filesystem permissions for profiles where Codex owns sandbox construction. -#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, Eq, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] #[ts(tag = "type")] pub enum ManagedFileSystemPermissions { @@ -262,6 +291,8 @@ pub enum ManagedFileSystemPermissions { #[serde(rename_all = "snake_case")] #[ts(rename_all = "snake_case")] Restricted { + #[schemars(with = "Vec")] + #[ts(as = "Vec")] entries: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] @@ -271,6 +302,68 @@ pub enum ManagedFileSystemPermissions { Unrestricted, } +#[derive(Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +enum SerializedManagedFileSystemPermissions { + #[serde(rename_all = "snake_case")] + Restricted { + entries: Vec, + #[serde(default, skip_serializing_if = "Option::is_none")] + glob_scan_max_depth: Option, + }, + Unrestricted, +} + +impl Serialize for ManagedFileSystemPermissions { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + match self { + Self::Restricted { + entries, + glob_scan_max_depth, + } => SerializedManagedFileSystemPermissions::Restricted { + entries: entries + .clone() + .into_iter() + .map(TryInto::try_into) + .collect::>() + .map_err(serde::ser::Error::custom)?, + glob_scan_max_depth: *glob_scan_max_depth, + } + .serialize(serializer), + Self::Unrestricted => { + SerializedManagedFileSystemPermissions::Unrestricted.serialize(serializer) + } + } + } +} + +impl<'de> Deserialize<'de> for ManagedFileSystemPermissions { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Ok( + match SerializedManagedFileSystemPermissions::deserialize(deserializer)? { + SerializedManagedFileSystemPermissions::Restricted { + entries, + glob_scan_max_depth, + } => Self::Restricted { + entries: entries + .into_iter() + .map(TryInto::try_into) + .collect::>() + .map_err(serde::de::Error::custom)?, + glob_scan_max_depth, + }, + SerializedManagedFileSystemPermissions::Unrestricted => Self::Unrestricted, + }, + ) + } +} + impl ManagedFileSystemPermissions { fn from_sandbox_policy(file_system_sandbox_policy: &FileSystemSandboxPolicy) -> Self { match file_system_sandbox_policy.kind { @@ -2654,6 +2747,33 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn permission_profile_round_trip_preserves_ambiguous_native_denies() -> Result<()> { + let denied_path = AbsolutePathBuf::try_from(PathBuf::from("/C:/secret"))?; + let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new(denied_path.into(), FileSystemAccessMode::Deny), + ]); + let permission_profile = PermissionProfile::from_runtime_permissions( + &file_system_sandbox_policy, + NetworkSandboxPolicy::Restricted, + ); + + assert_eq!( + serde_json::from_value::(serde_json::to_value( + &permission_profile + )?)?, + permission_profile + ); + Ok(()) + } + #[test] fn permission_profile_deserializes_legacy_rollout_shape() -> Result<()> { let legacy = serde_json::json!({ diff --git a/codex-rs/protocol/src/permissions.rs b/codex-rs/protocol/src/permissions.rs index ed21a2b9bf..11ead20443 100644 --- a/codex-rs/protocol/src/permissions.rs +++ b/codex-rs/protocol/src/permissions.rs @@ -6,6 +6,8 @@ use std::path::PathBuf; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::canonicalize_preserving_symlinks; +use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathUri; use globset::GlobBuilder; use globset::GlobMatcher; use schemars::JsonSchema; @@ -170,10 +172,20 @@ impl FileSystemSpecialPath { } } -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct FileSystemSandboxEntry { pub path: FileSystemPath, pub access: FileSystemAccessMode, + pub missing_path_behavior: Option, +} + +/// Serialized filesystem entry used at legacy string-based seams. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] +#[schemars(rename = "FileSystemSandboxEntry")] +#[ts(rename = "FileSystemSandboxEntry")] +pub struct RawFileSystemSandboxEntry { + pub path: RawFileSystemPath, + pub access: FileSystemAccessMode, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub missing_path_behavior: Option, @@ -219,14 +231,24 @@ pub enum FileSystemSandboxKind { ExternalSandbox, } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct FileSystemSandboxPolicy { + pub kind: FileSystemSandboxKind, + pub glob_scan_max_depth: Option, + pub entries: Vec, +} + +/// Serialized filesystem policy used at legacy string-based seams. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] +#[schemars(rename = "FileSystemSandboxPolicy")] +#[ts(rename = "FileSystemSandboxPolicy")] +pub struct RawFileSystemSandboxPolicy { pub kind: FileSystemSandboxKind, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub glob_scan_max_depth: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub entries: Vec, + pub entries: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -361,13 +383,10 @@ enum InvalidDenyReadGlobBehavior { ReturnError, } -#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] -#[serde(tag = "type", rename_all = "snake_case")] -#[ts(tag = "type")] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum FileSystemPath { Path { - // TODO(anp): Use PathUri once permission paths no longer require native-path rollout serialization. - path: AbsolutePathBuf, + path: PathUri, }, /// A git-style glob pattern. Pattern entries currently support /// FileSystemAccessMode::Deny only. @@ -379,12 +398,142 @@ pub enum FileSystemPath { }, } +/// Serialized filesystem path whose literal path variant preserves the raw +/// legacy string until an explicit seam conversion selects its meaning. +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] +#[serde(tag = "type", rename_all = "snake_case")] +#[ts(tag = "type")] +#[schemars(rename = "FileSystemPath")] +#[ts(rename = "FileSystemPath")] +pub enum RawFileSystemPath { + Path { + #[schemars(with = "AbsolutePathBuf")] + #[ts(type = "string")] + path: LegacyAppPathString, + }, + GlobPattern { + pattern: String, + }, + Special { + value: FileSystemSpecialPath, + }, +} + impl From for FileSystemPath { fn from(path: AbsolutePathBuf) -> Self { + Self::Path { path: path.into() } + } +} + +impl From for FileSystemPath { + fn from(path: PathUri) -> Self { Self::Path { path } } } +fn path_uri_from_raw(path: LegacyAppPathString) -> Result { + let native_path = + serde::de::value::StrDeserializer::::new(path.as_str()); + if let Ok(path) = AbsolutePathBuf::deserialize(native_path) { + return Ok(PathUri::from(path)); + } + + PathUri::try_from(path).map_err(|err| err.to_string()) +} + +fn raw_path_from_uri(path: PathUri) -> Result { + let raw_path = LegacyAppPathString::from(path.clone()); + if path_uri_from_raw(raw_path.clone()).as_ref() == Ok(&path) { + Ok(raw_path) + } else { + Err("permission path cannot be represented losslessly".to_string()) + } +} + +impl TryFrom for FileSystemPath { + type Error = String; + + fn try_from(path: RawFileSystemPath) -> Result { + Ok(match path { + RawFileSystemPath::Path { path } => Self::Path { + path: path_uri_from_raw(path)?, + }, + RawFileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, + RawFileSystemPath::Special { value } => Self::Special { value }, + }) + } +} + +impl TryFrom for RawFileSystemPath { + type Error = String; + + fn try_from(path: FileSystemPath) -> Result { + Ok(match path { + FileSystemPath::Path { path } => Self::Path { + path: raw_path_from_uri(path)?, + }, + FileSystemPath::GlobPattern { pattern } => Self::GlobPattern { pattern }, + FileSystemPath::Special { value } => Self::Special { value }, + }) + } +} + +impl TryFrom for FileSystemSandboxEntry { + type Error = String; + + fn try_from(entry: RawFileSystemSandboxEntry) -> Result { + Ok(Self { + path: entry.path.try_into()?, + access: entry.access, + missing_path_behavior: entry.missing_path_behavior, + }) + } +} + +impl TryFrom for RawFileSystemSandboxEntry { + type Error = String; + + fn try_from(entry: FileSystemSandboxEntry) -> Result { + Ok(Self { + path: entry.path.try_into()?, + access: entry.access, + missing_path_behavior: entry.missing_path_behavior, + }) + } +} + +impl TryFrom for FileSystemSandboxPolicy { + type Error = String; + + fn try_from(policy: RawFileSystemSandboxPolicy) -> Result { + Ok(Self { + kind: policy.kind, + glob_scan_max_depth: policy.glob_scan_max_depth, + entries: policy + .entries + .into_iter() + .map(TryInto::try_into) + .collect::>()?, + }) + } +} + +impl TryFrom for RawFileSystemSandboxPolicy { + type Error = String; + + fn try_from(policy: FileSystemSandboxPolicy) -> Result { + Ok(Self { + kind: policy.kind, + glob_scan_max_depth: policy.glob_scan_max_depth, + entries: policy + .entries + .into_iter() + .map(TryInto::try_into) + .collect::>()?, + }) + } +} + const PROJECT_ROOTS_GLOB_PATTERN_PREFIX: &str = "codex-project-roots://"; pub fn project_roots_glob_pattern(subpath: &Path) -> String { @@ -931,7 +1080,7 @@ impl FileSystemSandboxPolicy { for path in additional_writable_roots { if !self.entries.iter().any(|entry| { entry.access.can_write() - && matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == path) + && matches!(&entry.path, FileSystemPath::Path { path: existing } if existing == &PathUri::from_abs_path(path)) }) { self.entries.push(FileSystemSandboxEntry::new( path.clone().into(), @@ -1197,10 +1346,11 @@ impl FileSystemSandboxPolicy { FileSystemPath::GlobPattern { .. } => {} FileSystemPath::Path { path } => { if entry.access.can_write() { - if cwd_absolute.as_ref().is_some_and(|cwd| cwd == path) { + let path = path.to_abs_path()?; + if cwd_absolute.as_ref().is_some_and(|cwd| cwd == &path) { workspace_root_writable = true; } else { - writable_roots.push(path.clone()); + writable_roots.push(path); } } } @@ -1346,7 +1496,7 @@ fn resolve_file_system_path( cwd: Option<&AbsolutePathBuf>, ) -> Option { match path { - FileSystemPath::Path { path } => Some(path.clone()), + FileSystemPath::Path { path } => path.to_abs_path().ok(), FileSystemPath::GlobPattern { .. } => None, FileSystemPath::Special { value } => resolve_file_system_special_path(value, cwd), } @@ -1399,9 +1549,9 @@ fn file_system_paths_share_target(left: &FileSystemPath, right: &FileSystemPath) special_paths_share_target(left, right) } (FileSystemPath::Path { path }, FileSystemPath::Special { value }) - | (FileSystemPath::Special { value }, FileSystemPath::Path { path }) => { - special_path_matches_absolute_path(value, path) - } + | (FileSystemPath::Special { value }, FileSystemPath::Path { path }) => path + .to_abs_path() + .is_ok_and(|path| special_path_matches_absolute_path(value, &path)), ( FileSystemPath::GlobPattern { pattern: left }, FileSystemPath::GlobPattern { pattern: right }, @@ -1941,6 +2091,67 @@ mod tests { std::os::unix::fs::symlink(original, link) } + #[test] + fn permission_paths_preserve_native_strings_across_path_conventions() { + for path in [ + "/workspace/src", + r"C:\workspace\src", + r"\\server\share\src", + r"\\localhost\share", + ] { + let expected = serde_json::json!({ "type": "path", "path": path }); + let actual = serde_json::from_value::(expected.clone()) + .expect("valid raw permission path"); + assert_eq!( + serde_json::to_value(actual).expect("lossless raw permission path"), + expected + ); + } + } + + #[cfg(windows)] + #[test] + fn permission_paths_preserve_native_slash_unc_strings() { + for path in ["//server/share/src", r"/\server/share/src"] { + let expected = serde_json::json!({ "type": "path", "path": path }); + let actual = serde_json::from_value::(expected.clone()) + .expect("valid raw slash UNC permission path"); + assert_eq!( + serde_json::to_value(actual).expect("lossless raw slash UNC permission path"), + expected + ); + } + } + + #[cfg(unix)] + #[test] + fn native_ambiguous_permission_paths_keep_deny_semantics() { + let cwd = TempDir::new().expect("tempdir"); + for path in ["//server/share/secret", "/C:/secret"] { + let denied_path = serde_json::from_value::(serde_json::json!({ + "type": "path", + "path": path, + })) + .expect("raw permission path") + .try_into() + .expect("runtime permission path"); + let policy = FileSystemSandboxPolicy::restricted(vec![ + FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + ), + FileSystemSandboxEntry::new(denied_path, FileSystemAccessMode::Deny), + ]); + + assert!( + !policy.can_read_path_with_cwd(Path::new(path), cwd.path()), + "deny should apply to {path}" + ); + } + } + #[test] fn unknown_special_paths_are_ignored_by_legacy_bridge() -> std::io::Result<()> { let policy = FileSystemSandboxPolicy::restricted(vec![ @@ -2174,7 +2385,7 @@ mod tests { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: explicit_dot_codex.clone(), + path: explicit_dot_codex.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -2215,7 +2426,7 @@ mod tests { let root = AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute cwd"); let file_system_policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::Path { path: root }, + path: root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }]); @@ -2289,10 +2500,7 @@ mod tests { ) .into_iter() .map(|path| { - FileSystemSandboxEntry::skip_missing_path( - FileSystemPath::Path { path }, - FileSystemAccessMode::Read, - ) + FileSystemSandboxEntry::skip_missing_path(path.into(), FileSystemAccessMode::Read) }), ); @@ -2342,12 +2550,12 @@ mod tests { let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_root }, + path: link_root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_blocked }, + path: link_blocked.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -2412,7 +2620,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_blocked }, + path: link_blocked.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -2470,7 +2678,7 @@ mod tests { .expect("absolute canonical decoy"); let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { - path: FileSystemPath::Path { path: root }, + path: root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }]); @@ -2511,12 +2719,12 @@ mod tests { let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_root }, + path: link_root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_private }, + path: link_private.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -2560,12 +2768,12 @@ mod tests { let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_root }, + path: link_root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_private }, + path: link_private.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -2604,12 +2812,12 @@ mod tests { let policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { path: root }, + path: root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: alias }, + path: alias.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -2671,7 +2879,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: link_blocked }, + path: link_blocked.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -2713,20 +2921,20 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs.clone() }, + path: docs.clone().into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: docs_private.clone(), + path: docs_private.clone().into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: docs_private_public.clone(), + path: docs_private_public.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -2764,7 +2972,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs }, + path: docs.into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, @@ -2852,7 +3060,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs.clone() }, + path: docs.clone().into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, @@ -2892,7 +3100,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs.clone() }, + path: docs.clone().into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, @@ -2952,12 +3160,12 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs.clone() }, + path: docs.clone().into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs.clone() }, + path: docs.clone().into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, @@ -3035,7 +3243,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: extra }, + path: extra.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, @@ -3082,28 +3290,28 @@ mod tests { FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { - path: first.clone(), + path: first.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: second.clone(), + path: second.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: first.join(".git"), + path: first.join(".git").into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: second.join(".git"), + path: second.join(".git").into(), }, access: FileSystemAccessMode::Read, missing_path_behavior: None, @@ -3192,14 +3400,14 @@ mod tests { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: extra.clone() + path: extra.clone().into() }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry::skip_missing_path( FileSystemPath::Path { - path: extra.join(".git") + path: extra.join(".git").into() }, FileSystemAccessMode::Read, ), @@ -3218,7 +3426,7 @@ mod tests { let denied = AbsolutePathBuf::try_from("/tmp/private").expect("absolute path"); let existing = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: denied.clone(), + path: denied.clone().into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, @@ -3234,7 +3442,7 @@ mod tests { rebuilt.entries.iter().any(|entry| { entry.path == FileSystemPath::Path { - path: denied.clone(), + path: denied.clone().into(), } && entry.access == FileSystemAccessMode::Deny }), @@ -3268,7 +3476,9 @@ mod tests { fn deny_policy(path: &Path) -> FileSystemSandboxPolicy { FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::try_from(path).expect("absolute deny path"), + path: AbsolutePathBuf::try_from(path) + .expect("absolute deny path") + .into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 2ae44aea08..9dffc109c3 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -100,6 +100,7 @@ pub use crate::permissions::FileSystemSandboxKind; pub use crate::permissions::FileSystemSandboxPolicy; pub use crate::permissions::FileSystemSpecialPath; pub use crate::permissions::NetworkSandboxPolicy; +pub use crate::permissions::RawFileSystemSandboxPolicy; use crate::permissions::default_read_only_subpaths_for_writable_root; pub use crate::request_permissions::RequestPermissionsArgs; pub use crate::request_user_input::RequestUserInputEvent; @@ -1114,7 +1115,9 @@ impl FromStr for FileSystemSandboxPolicy { type Err = serde_json::Error; fn from_str(s: &str) -> Result { - serde_json::from_str(s) + serde_json::from_str::(s)? + .try_into() + .map_err(serde_json::Error::custom) } } @@ -3039,7 +3042,7 @@ pub struct TurnContextItem { #[serde(skip_serializing_if = "Option::is_none")] pub network: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub file_system_sandbox_policy: Option, + pub file_system_sandbox_policy: Option, pub model: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub comp_hash: Option, @@ -3066,8 +3069,13 @@ pub struct TurnContextItem { impl TurnContextItem { pub fn permission_profile(&self) -> PermissionProfile { self.permission_profile.clone().unwrap_or_else(|| { - let file_system_sandbox_policy = - self.file_system_sandbox_policy.clone().unwrap_or_else(|| { + let file_system_sandbox_policy = self + .file_system_sandbox_policy + .clone() + .map(TryInto::try_into) + .transpose() + .unwrap_or_else(|_| Some(FileSystemSandboxPolicy::restricted(Vec::new()))) + .unwrap_or_else(|| { FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd( &self.sandbox_policy, self.cwd.as_path(), @@ -4729,7 +4737,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: blocked }, + path: blocked.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -4789,7 +4797,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: secret }, + path: secret.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -4853,12 +4861,12 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs }, + path: docs.into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: docs_public }, + path: docs_public.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, @@ -4894,7 +4902,7 @@ mod tests { }; let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::Path { - path: external_write_path, + path: external_write_path.into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, @@ -5779,15 +5787,17 @@ mod tests { allowed_domains: vec!["api.example.com".to_string()], denied_domains: vec!["blocked.example.com".to_string()], }), - file_system_sandbox_policy: Some(FileSystemSandboxPolicy::restricted(vec![ - FileSystemSandboxEntry { + file_system_sandbox_policy: Some( + FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry { path: FileSystemPath::GlobPattern { pattern: "/tmp/private/**/*.txt".to_string(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, - }, - ])), + }]) + .try_into() + .expect("serializable split policy"), + ), model: "gpt-5".to_string(), comp_hash: None, personality: None, diff --git a/codex-rs/sandboxing/src/policy_transforms.rs b/codex-rs/sandboxing/src/policy_transforms.rs index 9117eced01..c723961c42 100644 --- a/codex-rs/sandboxing/src/policy_transforms.rs +++ b/codex-rs/sandboxing/src/policy_transforms.rs @@ -36,9 +36,12 @@ pub fn normalize_additional_permissions( } let path = match entry.path { FileSystemPath::Path { path } => FileSystemPath::Path { - path: canonicalize_preserving_symlinks(path.as_path()) + path: path + .to_abs_path() .ok() + .and_then(|path| canonicalize_preserving_symlinks(path.as_path()).ok()) .and_then(|path| AbsolutePathBuf::from_absolute_path(path).ok()) + .map(Into::into) .unwrap_or(path), }, FileSystemPath::GlobPattern { pattern } => { @@ -381,7 +384,7 @@ fn materialize_cwd_dependent_entry( fn resolve_permission_path(path: &FileSystemPath, cwd: &Path) -> Option { match path { - FileSystemPath::Path { path } => Some(path.clone()), + FileSystemPath::Path { path } => path.to_abs_path().ok(), FileSystemPath::GlobPattern { .. } => None, FileSystemPath::Special { value } => match value { FileSystemSpecialPath::Root => { diff --git a/codex-rs/sandboxing/src/seatbelt_tests.rs b/codex-rs/sandboxing/src/seatbelt_tests.rs index bfd440c50e..463b515a5b 100644 --- a/codex-rs/sandboxing/src/seatbelt_tests.rs +++ b/codex-rs/sandboxing/src/seatbelt_tests.rs @@ -132,7 +132,7 @@ fn filesystem_helper_platform_defaults_do_not_grant_applications_directory() { let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry::new( FileSystemPath::Path { - path: workspace_root, + path: workspace_root.into(), }, FileSystemAccessMode::Read, ), @@ -272,7 +272,7 @@ fn explicit_unreadable_paths_are_excluded_from_full_disk_read_and_write_access() missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: unreadable }, + path: unreadable.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, @@ -375,12 +375,12 @@ fn explicit_unreadable_paths_are_excluded_from_readable_roots() { let unreadable = absolute_path("/tmp/codex-readable/private"); let file_system_policy = FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { - path: FileSystemPath::Path { path: root }, + path: root.into(), access: FileSystemAccessMode::Read, missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: unreadable }, + path: unreadable.into(), access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, diff --git a/codex-rs/tui/src/additional_dirs.rs b/codex-rs/tui/src/additional_dirs.rs index cf7e087e0a..29cff6d3a8 100644 --- a/codex-rs/tui/src/additional_dirs.rs +++ b/codex-rs/tui/src/additional_dirs.rs @@ -116,7 +116,11 @@ mod tests { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: "/tmp/writable".try_into().expect("absolute path"), + path: codex_utils_absolute_path::AbsolutePathBuf::try_from( + "/tmp/writable", + ) + .expect("absolute path") + .into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index bb1da4bc59..99b1f3535d 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -2583,7 +2583,7 @@ mod tests { missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: extra_root }, + path: extra_root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index fc0aee23bf..da90e2dbd5 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -44,7 +44,7 @@ fn app_server_workspace_write_profile(extra_root: AbsolutePathBuf) -> Permission missing_path_behavior: None, }, FileSystemSandboxEntry { - path: FileSystemPath::Path { path: extra_root }, + path: extra_root.into(), access: FileSystemAccessMode::Write, missing_path_behavior: None, }, @@ -368,7 +368,7 @@ async fn preset_matching_does_not_treat_non_cwd_writable_profile_as_read_only() }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: test_path_buf("/tmp/writable").abs(), + path: test_path_buf("/tmp/writable").abs().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/tui/src/permission_compat.rs b/codex-rs/tui/src/permission_compat.rs index e207db0c4a..8922f6c243 100644 --- a/codex-rs/tui/src/permission_compat.rs +++ b/codex-rs/tui/src/permission_compat.rs @@ -69,7 +69,7 @@ mod tests { }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: extra_root.clone(), + path: extra_root.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, diff --git a/codex-rs/utils/path-uri/src/api_path_string_tests.rs b/codex-rs/utils/path-uri/src/api_path_string_tests.rs index 8fe6a7c085..aecbad8edb 100644 --- a/codex-rs/utils/path-uri/src/api_path_string_tests.rs +++ b/codex-rs/utils/path-uri/src/api_path_string_tests.rs @@ -82,8 +82,8 @@ const RENDER_CASES: &[RenderCase] = &[ ), RenderCase::round_trips("file:///etc", PathConvention::Posix, "/etc"), RenderCase::round_trips("file:///tmp/", PathConvention::Posix, "/tmp/"), - RenderCase::round_trips("file:///C:/Project", PathConvention::Posix, "/C:/Project"), - RenderCase::round_trips("file:///C:", PathConvention::Posix, "/C:"), + RenderCase::renders_lossily("file:///C:/Project", PathConvention::Posix, "/C:/Project"), + RenderCase::renders_lossily("file:///C:", PathConvention::Posix, "/C:"), RenderCase::round_trips("file:///tmp/%E2%98%83", PathConvention::Posix, "/tmp/☃"), RenderCase::round_trips("file:///tmp/a%5Cb", PathConvention::Posix, "/tmp/a\\b"), RenderCase::round_trips( @@ -466,6 +466,19 @@ fn converts_absolute_api_paths_using_the_inferred_convention() { } } +#[test] +fn ambiguous_absolute_api_paths_preserve_their_inferred_convention() { + for (raw_path, convention) in [ + ("/C:/secret", PathConvention::Posix), + (r"\\localhost\share", PathConvention::Windows), + ] { + let path = LegacyAppPathString::from_string(raw_path); + let uri = PathUri::try_from(path.clone()).expect("absolute API path should convert"); + assert_eq!(uri.infer_path_convention(), Some(convention)); + assert_eq!(LegacyAppPathString::from(uri), path); + } +} + #[test] fn converts_native_api_path_to_inferred_absolute_path() { #[cfg(windows)] diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 51590b568e..7dd24c2c67 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -106,7 +106,8 @@ impl PathUri { /// Paths without a valid URI representation are replaced by /// `file:///%00/bad/path/`, where `` is the URL-safe, unpadded /// encoding of the original path (Unix bytes or Windows UTF-16LE). This - /// includes paths containing nulls and, on Windows, unsupported prefix + /// includes paths containing nulls, paths whose URI spelling would imply a + /// different convention, and, on Windows, unsupported prefix /// kinds such as device and generic verbatim namespaces, non-Unicode path /// or UNC components, and UNC server names that are not valid URL hosts. /// The encoded null reserves a URI namespace that cannot collide with a @@ -114,6 +115,8 @@ impl PathUri { pub fn from_abs_path(path: &AbsolutePathBuf) -> Self { if let Ok(url) = Url::from_file_path(path.as_path()) && let Ok(uri) = Self::try_from(url) + && uri.0.host_str() != Some("") + && uri.infer_path_convention() == Some(PathConvention::native()) { return uri; } @@ -135,15 +138,24 @@ impl PathUri { Self::from_opaque_path_bytes(&path_bytes) } - /// Parses an absolute native path using the specified path convention. + /// Parses an absolute native path using the specified path convention, + /// falling back to an opaque URI when its ordinary URI spelling would + /// imply a different convention. pub(crate) fn from_absolute_native_path( path: &str, convention: PathConvention, ) -> Option { - match convention { + let uri = match convention { PathConvention::Posix => parse_posix_path(path), PathConvention::Windows => parse_windows_path(path), + }?; + if uri.0.host_str() != Some("") && uri.infer_path_convention() == Some(convention) { + return Some(uri); } + Some(match convention { + PathConvention::Posix => Self::from_opaque_path_bytes(path.as_bytes()), + PathConvention::Windows => windows_opaque_path_uri(path), + }) } fn from_opaque_path_bytes(path_bytes: &[u8]) -> Self { diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index 7117a0c3bc..a2a1e80564 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -480,6 +480,15 @@ fn file_uri_round_trips_windows_unc_paths() { assert_eq!(uri.encoded_path(), "/share/src/main.rs"); assert_eq!(uri.to_abs_path().expect("UNC URI should convert"), path); + + let localhost = AbsolutePathBuf::from_absolute_path_checked(r"\\localhost\share\src") + .expect("absolute localhost UNC path"); + let uri = PathUri::from_abs_path(&localhost); + assert!(uri.to_string().starts_with(BAD_PATH_URI_PREFIX)); + assert_eq!( + uri.to_abs_path().expect("opaque URI should convert"), + localhost + ); } #[test] diff --git a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs index b3ad915419..498ceabfd0 100644 --- a/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs +++ b/codex-rs/windows-sandbox-rs/src/deny_read_resolver.rs @@ -226,7 +226,9 @@ mod tests { fn unreadable_path_entry(path: PathBuf) -> FileSystemSandboxEntry { FileSystemSandboxEntry { path: FileSystemPath::Path { - path: AbsolutePathBuf::from_absolute_path(path).expect("absolute path"), + path: AbsolutePathBuf::from_absolute_path(path) + .expect("absolute path") + .into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, diff --git a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs index d924c5c112..36e9342eef 100644 --- a/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs +++ b/codex-rs/windows-sandbox-rs/src/resolved_permissions.rs @@ -333,28 +333,28 @@ mod tests { FileSystemSandboxPolicy::restricted(vec![ FileSystemSandboxEntry { path: FileSystemPath::Path { - path: first.clone(), + path: first.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: second.clone(), + path: second.clone().into(), }, access: FileSystemAccessMode::Write, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: first.join(".git"), + path: first.join(".git").into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None, }, FileSystemSandboxEntry { path: FileSystemPath::Path { - path: second.join(".git"), + path: second.join(".git").into(), }, access: FileSystemAccessMode::Deny, missing_path_behavior: None,