From df0f70f7b3672c6b61ea098219a6d13e6325cc5f Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 10 Jun 2026 05:47:25 +0000 Subject: [PATCH] Migrate ExecutorFileSystem read_file to PathUri --- codex-rs/Cargo.lock | 6 + codex-rs/app-server/Cargo.toml | 1 + .../src/request_processors/fs_processor.rs | 4 +- codex-rs/apply-patch/Cargo.toml | 1 + codex-rs/apply-patch/src/invocation.rs | 14 +- codex-rs/apply-patch/src/lib.rs | 21 +- codex-rs/config/Cargo.toml | 2 +- codex-rs/config/src/loader/layer_io.rs | 4 +- codex-rs/config/src/loader/mod.rs | 13 +- codex-rs/config/src/loader/tests.rs | 3 +- codex-rs/core-skills/src/injection.rs | 20 +- codex-rs/core-skills/src/loader.rs | 19 +- codex-rs/core-skills/src/model.rs | 5 +- codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/agents_md.rs | 18 +- codex-rs/core/src/config/agent_roles.rs | 4 +- codex-rs/core/src/config/mod.rs | 4 +- .../core/src/tools/handlers/view_image.rs | 9 +- codex-rs/core/tests/common/Cargo.toml | 1 + codex-rs/core/tests/common/test_codex.rs | 5 +- codex-rs/core/tests/suite/apply_patch_cli.rs | 6 +- codex-rs/core/tests/suite/remote_env.rs | 31 ++- codex-rs/exec-server/src/environment.rs | 1 + codex-rs/exec-server/src/fs_helper.rs | 4 +- codex-rs/exec-server/src/local_file_system.rs | 11 +- .../src/local_file_system_path_uri_tests.rs | 27 +++ .../exec-server/src/remote_file_system.rs | 9 +- .../src/remote_file_system_path_uri_tests.rs | 225 ++++++++++++++++++ .../exec-server/src/sandboxed_file_system.rs | 8 +- .../sandboxed_file_system_path_uri_tests.rs | 43 ++++ .../src/server/file_system_handler.rs | 3 +- .../exec-server/tests/file_system/shared.rs | 7 +- .../exec-server/tests/file_system_unix.rs | 8 +- codex-rs/ext/skills/Cargo.toml | 2 +- codex-rs/ext/skills/src/provider/executor.rs | 9 +- .../tests/executor_file_system_authority.rs | 4 +- codex-rs/file-system/src/lib.rs | 4 +- codex-rs/git-utils/Cargo.toml | 1 + codex-rs/git-utils/src/info.rs | 7 +- codex-rs/utils/plugins/Cargo.toml | 1 + .../utils/plugins/src/plugin_namespace.rs | 4 +- 41 files changed, 509 insertions(+), 61 deletions(-) create mode 100644 codex-rs/exec-server/src/local_file_system_path_uri_tests.rs create mode 100644 codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs create mode 100644 codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 46dccdeae7..24be6034a7 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2004,6 +2004,7 @@ dependencies = [ "codex-utils-cargo-bin", "codex-utils-cli", "codex-utils-json-to-toml", + "codex-utils-path-uri", "codex-utils-pty", "codex-web-search-extension", "codex-windows-sandbox", @@ -2183,6 +2184,7 @@ dependencies = [ "codex-exec-server", "codex-utils-absolute-path", "codex-utils-cargo-bin", + "codex-utils-path-uri", "pretty_assertions", "similar", "tempfile", @@ -2635,6 +2637,7 @@ dependencies = [ "codex-utils-image", "codex-utils-output-truncation", "codex-utils-path", + "codex-utils-path-uri", "codex-utils-plugins", "codex-utils-pty", "codex-utils-stream-parser", @@ -3031,6 +3034,7 @@ dependencies = [ "codex-file-system", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "futures", "gix", "once_cell", @@ -4201,6 +4205,7 @@ dependencies = [ "codex-exec-server", "codex-login", "codex-utils-absolute-path", + "codex-utils-path-uri", "serde", "serde_json", "tempfile", @@ -4590,6 +4595,7 @@ dependencies = [ "codex-protocol", "codex-utils-absolute-path", "codex-utils-cargo-bin", + "codex-utils-path-uri", "ctor 0.6.3", "futures", "notify", diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 55ea78e617..442fb5df29 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -76,6 +76,7 @@ codex-thread-store = { workspace = true } codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-json-to-toml = { workspace = true } +codex-utils-path-uri = { workspace = true } chrono = { workspace = true } clap = { workspace = true, features = ["derive"] } futures = { workspace = true } diff --git a/codex-rs/app-server/src/request_processors/fs_processor.rs b/codex-rs/app-server/src/request_processors/fs_processor.rs index 99a8620b4f..cd46f8600f 100644 --- a/codex-rs/app-server/src/request_processors/fs_processor.rs +++ b/codex-rs/app-server/src/request_processors/fs_processor.rs @@ -29,6 +29,7 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; +use codex_utils_path_uri::PathUri; use std::io; use std::sync::Arc; @@ -64,9 +65,10 @@ impl FsRequestProcessor { &self, params: FsReadFileParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let bytes = self .file_system()? - .read_file(¶ms.path, /*sandbox*/ None) + .read_file(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsReadFileResponse { diff --git a/codex-rs/apply-patch/Cargo.toml b/codex-rs/apply-patch/Cargo.toml index 2584338618..8e1d7f651f 100644 --- a/codex-rs/apply-patch/Cargo.toml +++ b/codex-rs/apply-patch/Cargo.toml @@ -20,6 +20,7 @@ workspace = true anyhow = { workspace = true } codex-exec-server = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } similar = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["macros", "rt"] } diff --git a/codex-rs/apply-patch/src/invocation.rs b/codex-rs/apply-patch/src/invocation.rs index 1aa429e0d3..b66ee2291d 100644 --- a/codex-rs/apply-patch/src/invocation.rs +++ b/codex-rs/apply-patch/src/invocation.rs @@ -21,6 +21,7 @@ use crate::parser::Hunk; use crate::parser::ParseError; use crate::parser::parse_patch; use crate::unified_diff_from_chunks; +use codex_utils_path_uri::PathUri; use std::str::Utf8Error; use tree_sitter::LanguageError; @@ -185,7 +186,18 @@ pub async fn verify_apply_patch_args( ); } Hunk::DeleteFile { .. } => { - let content = match fs.read_file_text(&path, sandbox).await { + let path_uri = match PathUri::from_abs_path(&path) { + Ok(path_uri) => path_uri, + Err(e) => { + return MaybeApplyPatchVerified::CorrectnessError( + ApplyPatchError::IoError(IoError { + context: format!("Failed to read {}", path.display()), + source: e, + }), + ); + } + }; + let content = match fs.read_file_text(&path_uri, sandbox).await { Ok(content) => content, Err(e) => { return MaybeApplyPatchVerified::CorrectnessError( diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 29d42e1c07..ec6aa1930f 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -16,6 +16,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::RemoveOptions; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; pub use parser::Hunk; pub use parser::ParseError; use parser::ParseError::*; @@ -390,6 +391,7 @@ async fn apply_hunks_to_files( for hunk in hunks { let affected_path = hunk.path().to_path_buf(); let path_abs = hunk.resolve_path(cwd); + let path_uri = PathUri::from_abs_path(&path_abs)?; match hunk { Hunk::AddFile { contents, .. } => { let overwritten_content = @@ -415,7 +417,7 @@ async fn apply_hunks_to_files( } Hunk::DeleteFile { .. } => { note_existing_path_delta_support(&path_abs, fs, sandbox, &mut delta.exact).await; - let deleted_content = fs.read_file_text(&path_abs, sandbox).await.ok(); + let deleted_content = fs.read_file_text(&path_uri, sandbox).await.ok(); if deleted_content.is_none() { delta.exact = false; } @@ -570,9 +572,12 @@ async fn remove_failure_was_side_effect_free( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> bool { + let Ok(path_uri) = PathUri::from_abs_path(path) else { + return false; + }; match expected_content { Some(expected_content) => fs - .read_file_text(path, sandbox) + .read_file_text(&path_uri, sandbox) .await .is_ok_and(|content| content == expected_content), None => false, @@ -586,7 +591,14 @@ async fn read_optional_file_text_for_delta( exact: &mut bool, ) -> Option { note_existing_path_delta_support(path, fs, sandbox, exact).await; - match fs.read_file_text(path, sandbox).await { + let path_uri = match PathUri::from_abs_path(path) { + Ok(path_uri) => path_uri, + Err(_) => { + *exact = false; + return None; + } + }; + match fs.read_file_text(&path_uri, sandbox).await { Ok(content) => Some(content), Err(source) if source.kind() == io::ErrorKind::NotFound => None, Err(_) => { @@ -657,7 +669,8 @@ async fn derive_new_contents_from_chunks( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> std::result::Result { - let original_contents = fs.read_file_text(path_abs, sandbox).await.map_err(|err| { + let path_uri = PathUri::from_abs_path(path_abs)?; + let original_contents = fs.read_file_text(&path_uri, sandbox).await.map_err(|err| { ApplyPatchError::IoError(IoError { context: format!("Failed to read file to update {}", path_abs.display()), source: err, diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index 31e6be005a..e2c650dccc 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -25,6 +25,7 @@ codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-path = { workspace = true } +codex-utils-path-uri = { workspace = true } dunce = { workspace = true } futures = { workspace = true, features = ["alloc", "std"] } gethostname = { workspace = true } @@ -62,7 +63,6 @@ windows-sys = { version = "0.52", features = [ ] } [dev-dependencies] -codex-utils-path-uri = { workspace = true } pretty_assertions = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true, features = ["full"] } diff --git a/codex-rs/config/src/loader/layer_io.rs b/codex-rs/config/src/loader/layer_io.rs index 415d82e405..7cdb4f5359 100644 --- a/codex-rs/config/src/loader/layer_io.rs +++ b/codex-rs/config/src/loader/layer_io.rs @@ -10,6 +10,7 @@ use crate::strict_config::config_error_from_ignored_toml_value_fields; use codex_file_system::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use std::io; use std::path::Path; use std::path::PathBuf; @@ -106,7 +107,8 @@ pub(super) async fn read_config_from_path( log_missing_as_info: bool, strict_config: bool, ) -> io::Result> { - match fs.read_file_text(path, /*sandbox*/ None).await { + let path_uri = PathUri::from_abs_path(path)?; + match fs.read_file_text(&path_uri, /*sandbox*/ None).await { Ok(contents) => match toml::from_str::(&contents) { Ok(value) => { if strict_config { diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 1a79e984e3..9501d67250 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -40,6 +40,7 @@ use codex_protocol::config_types::TrustLevel; use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use dunce::canonicalize as normalize_path; use serde::Deserialize; use std::io; @@ -471,7 +472,8 @@ async fn load_config_toml_for_required_layer( strict_config: bool, create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry, ) -> io::Result { - let toml_value = match fs.read_file_text(toml_file, /*sandbox*/ None).await { + let toml_file_uri = PathUri::from_abs_path(toml_file)?; + let toml_value = match fs.read_file_text(&toml_file_uri, /*sandbox*/ None).await { Ok(contents) => { let config_parent = toml_file.as_path().parent().ok_or_else(|| { io::Error::new( @@ -566,8 +568,9 @@ pub async fn load_requirements_toml( fs: &dyn ExecutorFileSystem, requirements_toml_file: &AbsolutePathBuf, ) -> io::Result> { + let requirements_toml_file_uri = PathUri::from_abs_path(requirements_toml_file)?; match fs - .read_file_text(requirements_toml_file, /*sandbox*/ None) + .read_file_text(&requirements_toml_file_uri, /*sandbox*/ None) .await { Ok(contents) => { @@ -1224,7 +1227,8 @@ async fn load_project_layers( continue; } let config_file = dot_codex_abs.join(CONFIG_TOML_FILE); - match fs.read_file_text(&config_file, /*sandbox*/ None).await { + let config_file_uri = PathUri::from_abs_path(&config_file)?; + match fs.read_file_text(&config_file_uri, /*sandbox*/ None).await { Ok(contents) => { let config: TomlValue = match toml::from_str(&contents) { Ok(config) => config, @@ -1327,8 +1331,9 @@ async fn merge_root_checkout_project_hooks( return Ok(config); }; let hooks_config_file = hooks_config_folder.join(CONFIG_TOML_FILE); + let hooks_config_file_uri = PathUri::from_abs_path(&hooks_config_file)?; let root_config = match fs - .read_file_text(&hooks_config_file, /*sandbox*/ None) + .read_file_text(&hooks_config_file_uri, /*sandbox*/ None) .await { Ok(contents) => { diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 7dc9b13bd0..23ea782742 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -27,9 +27,10 @@ impl ExecutorFileSystem for TestFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { + let path = path.to_abs_path()?; tokio::fs::read(path.as_path()).await } diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index a358ddc637..03bb0a6ddd 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -13,6 +13,7 @@ use codex_exec_server::LOCAL_FS; use codex_otel::SessionTelemetry; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; #[derive(Debug, Default)] @@ -75,10 +76,21 @@ pub async fn build_skill_injections( let fs = loaded_skills .and_then(|outcome| outcome.file_system_for_skill(skill)) .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - match fs - .read_file_text(&skill.path_to_skills_md, /*sandbox*/ None) - .await - { + // Skill metadata may point at a file that is absent, but a path the host + // cannot represent means the configured skill cannot be loaded correctly. + let path = match PathUri::from_abs_path(&skill.path_to_skills_md) { + Ok(path) => path, + Err(err) => { + emit_skill_injected_metric(otel, skill, "error"); + result.warnings.push(format!( + "Failed to load skill {name} at {path}: {err:#}", + name = skill.name, + path = skill.path_to_skills_md.display() + )); + continue; + } + }; + match fs.read_file_text(&path, /*sandbox*/ None).await { Ok(contents) => { emit_skill_injected_metric(otel, skill, "ok"); invocations.push(SkillInvocation { diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index d4dc8b3b73..5a3c218007 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -640,8 +640,9 @@ async fn parse_skill_file( plugin_id: Option<&str>, plugin_root: Option<&AbsolutePathBuf>, ) -> Result { + let path_uri = PathUri::from_abs_path(path).map_err(SkillParseError::Read)?; let contents = fs - .read_file_text(path, /*sandbox*/ None) + .read_file_text(&path_uri, /*sandbox*/ None) .await .map_err(SkillParseError::Read)?; @@ -735,6 +736,17 @@ async fn load_skill_metadata( let metadata_path = skill_dir .join(SKILLS_METADATA_DIR) .join(SKILLS_METADATA_FILENAME); + let metadata_path_uri = match PathUri::from_abs_path(&metadata_path) { + Ok(path) => path, + Err(error) => { + tracing::warn!( + "ignoring {path}: failed to convert {label} path to URI: {error}", + path = metadata_path.display(), + label = SKILLS_METADATA_FILENAME + ); + return LoadedSkillMetadata::default(); + } + }; match fs.get_metadata(&metadata_path, /*sandbox*/ None).await { Ok(metadata) if metadata.is_file => {} Ok(_) => return LoadedSkillMetadata::default(), @@ -751,7 +763,10 @@ async fn load_skill_metadata( } } - let contents = match fs.read_file_text(&metadata_path, /*sandbox*/ None).await { + let contents = match fs + .read_file_text(&metadata_path_uri, /*sandbox*/ None) + .await + { Ok(contents) => contents, Err(error) => { tracing::warn!( diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index a9cc24306a..5569655adf 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -9,6 +9,7 @@ use codex_exec_server::LOCAL_FS; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; #[derive(Debug, Clone, PartialEq)] pub struct SkillMetadata { @@ -152,8 +153,8 @@ impl HostLoadedSkills { .outcome .file_system_for_skill(skill) .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - fs.read_file_text(&skill.path_to_skills_md, /*sandbox*/ None) - .await + let path = PathUri::from_abs_path(&skill.path_to_skills_md)?; + fs.read_file_text(&path, /*sandbox*/ None).await } } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 352e8051e4..60678db5dc 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -70,6 +70,7 @@ codex-utils-image = { workspace = true } codex-utils-home-dir = { workspace = true } codex-utils-output-truncation = { workspace = true } codex-utils-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-string = { workspace = true } diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 5739e42f30..70b51fb32a 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -26,6 +26,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_features::Feature; use codex_prompts::HIERARCHICAL_AGENTS_MESSAGE; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::io; use toml::Value as TomlValue; use tracing::error; @@ -58,7 +59,19 @@ impl<'a> AgentsMdManager<'a> { let base = codex_dir?; for candidate in [LOCAL_AGENTS_MD_FILENAME, DEFAULT_AGENTS_MD_FILENAME] { let path = base.join(candidate); - let data = match fs.read_file(&path, /*sandbox*/ None).await { + // A missing global instructions file is normal, but an unrepresentable + // configured path means Codex cannot honor the workspace configuration. + let path_uri = match PathUri::from_abs_path(&path) { + Ok(path_uri) => path_uri, + Err(err) => { + startup_warnings.push(format!( + "Failed to read global AGENTS.md instructions from `{}`: {err}", + path.display() + )); + continue; + } + }; + let data = match fs.read_file(&path_uri, /*sandbox*/ None).await { Ok(data) => data, Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) if err.kind() == io::ErrorKind::IsADirectory => continue, @@ -149,6 +162,7 @@ impl<'a> AgentsMdManager<'a> { break; } + let path_uri = PathUri::from_abs_path(&p)?; match fs.get_metadata(&p, /*sandbox*/ None).await { Ok(metadata) if !metadata.is_file => continue, Ok(_) => {} @@ -156,7 +170,7 @@ impl<'a> AgentsMdManager<'a> { Err(err) => return Err(err), } - let mut data = match fs.read_file(&p, /*sandbox*/ None).await { + let mut data = match fs.read_file(&path_uri, /*sandbox*/ None).await { Ok(data) => data, Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) => return Err(err), diff --git a/codex-rs/core/src/config/agent_roles.rs b/codex-rs/core/src/config/agent_roles.rs index abdef33e7d..269f5a54cd 100644 --- a/codex-rs/core/src/config/agent_roles.rs +++ b/codex-rs/core/src/config/agent_roles.rs @@ -7,6 +7,7 @@ use codex_config::config_toml::ConfigToml; use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use serde::Deserialize; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -319,7 +320,8 @@ async fn read_resolved_agent_role_file( path: &AbsolutePathBuf, role_name_hint: Option<&str>, ) -> std::io::Result { - let contents = fs.read_file_text(path, /*sandbox*/ None).await?; + let path_uri = PathUri::from_abs_path(path)?; + let contents = fs.read_file_text(&path_uri, /*sandbox*/ None).await?; let config_base_dir = path.parent().unwrap_or_else(|| path.clone()); parse_agent_role_file_contents( &contents, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index cadb797212..85fe9967f3 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -105,6 +105,7 @@ use codex_protocol::protocol::SandboxPolicy; pub use codex_thread_store::ExtraConfig; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use rmcp::model::ElicitationCapability; use rmcp::model::FormElicitationCapability; use rmcp::model::UrlElicitationCapability; @@ -3651,8 +3652,9 @@ impl Config { return Ok(None); }; + let path_uri = PathUri::from_abs_path(path)?; let contents = fs - .read_file_text(path, /*sandbox*/ None) + .read_file_text(&path_uri, /*sandbox*/ None) .await .map_err(|e| { std::io::Error::new( diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 0432ad338f..8f7dd509f9 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -25,6 +25,7 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; +use codex_utils_path_uri::PathUri; pub struct ViewImageHandler { options: ViewImageToolOptions, @@ -144,6 +145,12 @@ impl ViewImageHandler { let abs_path = cwd.join(path); let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, &cwd); let fs = turn_environment.environment.get_filesystem(); + let path_uri = PathUri::from_abs_path(&abs_path).map_err(|error| { + FunctionCallError::RespondToModel(format!( + "unable to locate image at `{}`: {error}", + abs_path.display() + )) + })?; let metadata = fs .get_metadata(&abs_path, Some(&sandbox)) @@ -162,7 +169,7 @@ impl ViewImageHandler { ))); } let file_bytes = fs - .read_file(&abs_path, Some(&sandbox)) + .read_file(&path_uri, Some(&sandbox)) .await .map_err(|error| { FunctionCallError::RespondToModel(format!( diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index fb6221cfce..575985d2c2 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -28,6 +28,7 @@ codex-models-manager = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cargo-bin = { workspace = true } +codex-utils-path-uri = { workspace = true } ctor = { workspace = true } futures = { workspace = true } notify = { workspace = true } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 62f422ba1c..56826692f5 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -43,6 +43,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use serde_json::Value; use tempfile::TempDir; @@ -924,10 +925,12 @@ impl TestCodexHarness { } pub async fn read_file_text(&self, rel: impl AsRef) -> Result { + let path = self.path_abs(rel); + let path_uri = PathUri::from_path(&path)?; Ok(self .test .fs() - .read_file_text(&self.path_abs(rel), /*sandbox*/ None) + .read_file_text(&path_uri, /*sandbox*/ None) .await?) } diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index a9ebadcebc..eaffa2dc2f 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -38,6 +38,7 @@ use codex_protocol::user_input::UserInput; #[cfg(target_os = "linux")] use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::assert_regex_match; use core_test_support::get_remote_test_env; @@ -1683,7 +1684,10 @@ async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Re assert_eq!(fs::read_to_string(shared_cwd.join(file_name))?, "local\n"); assert_eq!( test.fs() - .read_file_text(&shared_cwd.join(file_name), /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(shared_cwd.join(file_name))?, + /*sandbox*/ None, + ) .await?, "remote\n" ); diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 00f5a0c514..4508e3b75e 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -28,6 +28,7 @@ use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::get_remote_test_env; @@ -155,13 +156,14 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> { let file_system = test_env.environment().get_filesystem(); let file_path_abs = remote_test_file_path().abs(); + let file_path_uri = PathUri::from_path(&file_path_abs)?; let payload = b"remote-test-env-ok".to_vec(); file_system .write_file(&file_path_abs, payload.clone(), /*sandbox*/ None) .await?; let actual = file_system - .read_file(&file_path_abs, /*sandbox*/ None) + .read_file(&file_path_uri, /*sandbox*/ None) .await?; assert_eq!(actual, payload); @@ -527,7 +529,10 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result ); assert_eq!( test.fs() - .read_file_text(&remote_target_path, /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(&remote_target_path)?, + /*sandbox*/ None, + ) .await?, "remote-request-permissions-ok" ); @@ -610,7 +615,10 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result< let remote_contents = test .fs() - .read_file_text(&remote_cwd.join(file_name), /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(remote_cwd.join(file_name))?, + /*sandbox*/ None, + ) .await?; assert_eq!(remote_contents, "patched remote freeform\n"); assert!( @@ -664,6 +672,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() )) .abs(); + let target_path_uri = PathUri::from_path(&target_path)?; let _ = fs::remove_file(&target_path); test.fs() .remove( @@ -771,7 +780,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { .await; assert_eq!( test.fs() - .read_file_text(&target_path, /*sandbox*/ None) + .read_file_text(&target_path_uri, /*sandbox*/ None) .await?, "remote\n" ); @@ -785,7 +794,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { wait_for_completion_without_patch_approval(&test).await; assert_eq!( test.fs() - .read_file_text(&target_path, /*sandbox*/ None) + .read_file_text(&target_path_uri, /*sandbox*/ None) .await?, "remote updated\n" ); @@ -885,7 +894,10 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm let remote_contents = test .fs() - .read_file_text(&remote_cwd.join(file_name), /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(remote_cwd.join(file_name))?, + /*sandbox*/ None, + ) .await?; assert_eq!(remote_contents, "patched remote exec\n"); assert!( @@ -936,7 +948,7 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> { let sandbox = read_only_sandbox(allowed_dir.clone()); let contents = file_system - .read_file(&absolute_path(file_path.clone()), Some(&sandbox)) + .read_file(&PathUri::from_path(&file_path)?, Some(&sandbox)) .await?; assert_eq!(contents, b"sandboxed hello"); @@ -976,7 +988,8 @@ async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() - secret = secret_path.display(), ))?; - let requested_path = absolute_path(allowed_dir.join("link").join("..").join("secret.txt")); + let requested_path = + PathUri::from_path(allowed_dir.join("link").join("..").join("secret.txt"))?; let sandbox = read_only_sandbox(allowed_dir.clone()); let error = match file_system.read_file(&requested_path, Some(&sandbox)).await { Ok(_) => anyhow::bail!("read should fail after path normalization"), @@ -1038,7 +1051,7 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> { .is_ok(); assert!(!symlink_exists); let outside = file_system - .read_file_text(&absolute_path(outside_file.clone()), /*sandbox*/ None) + .read_file_text(&PathUri::from_path(&outside_file)?, /*sandbox*/ None) .await?; assert_eq!(outside, "outside"); diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 1dfd49d3fd..bc100b9422 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -882,6 +882,7 @@ mod tests { std::env::current_exe().expect("current exe").as_path(), ) .expect("absolute current exe"); + let path = codex_utils_path_uri::PathUri::from_abs_path(&path).expect("path URI"); let sandbox = crate::FileSystemSandboxContext::from_permission_profile( codex_protocol::models::PermissionProfile::from_runtime_permissions( &codex_protocol::permissions::FileSystemSandboxPolicy::restricted(Vec::new()), diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index ebda194265..1e77a3c4c9 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -189,8 +189,10 @@ pub(crate) async fn run_direct_request( let file_system = DirectFileSystem; match request { FsHelperRequest::ReadFile(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let data = file_system - .read_file(¶ms.path, /*sandbox*/ None) + .read_file(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsHelperPayload::ReadFile(FsReadFileResponse { diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 5a5f6d0595..63f94def8a 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -91,7 +91,7 @@ impl ExecutorFileSystem for LocalFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { let (file_system, sandbox) = self.file_system_for(sandbox)?; @@ -173,7 +173,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { reject_platform_sandbox_context(sandbox)?; @@ -271,10 +271,11 @@ impl ExecutorFileSystem for DirectFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; let metadata = tokio::fs::metadata(path.as_path()).await?; if metadata.len() > MAX_READ_FILE_BYTES { return Err(io::Error::new( @@ -599,3 +600,7 @@ mod tests { Ok(()) } } + +#[cfg(all(test, any(unix, windows)))] +#[path = "local_file_system_path_uri_tests.rs"] +mod path_uri_tests; diff --git a/codex-rs/exec-server/src/local_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/local_file_system_path_uri_tests.rs new file mode 100644 index 0000000000..dcddd98e08 --- /dev/null +++ b/codex-rs/exec-server/src/local_file_system_path_uri_tests.rs @@ -0,0 +1,27 @@ +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; +use tokio::io; + +use super::*; + +#[tokio::test] +async fn direct_file_system_rejects_non_native_uri_as_invalid_input() { + let error = DirectFileSystem + .read_file(&non_native_uri(), /*sandbox*/ None) + .await + .expect_err("non-native URI should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + +fn non_native_uri() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/file.txt"; + #[cfg(windows)] + let uri = "file:///usr/local/file.txt"; + + match PathUri::parse(uri) { + Ok(uri) => uri, + Err(err) => panic!("valid non-native URI should parse: {err}"), + } +} diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index f5b04e39df..e7afba6161 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -61,14 +61,15 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_file"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_read_file(FsReadFileParams { - path: path.clone(), + path, sandbox: remote_sandbox_context(sandbox), }) .await @@ -322,3 +323,7 @@ mod tests { AbsolutePathBuf::from_absolute_path(&path).expect("absolute path") } } + +#[cfg(all(test, any(unix, windows)))] +#[path = "remote_file_system_path_uri_tests.rs"] +mod path_uri_tests; diff --git a/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs new file mode 100644 index 0000000000..29d0edaffb --- /dev/null +++ b/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs @@ -0,0 +1,225 @@ +#![allow(clippy::expect_used)] + +#[cfg(windows)] +use codex_app_server_protocol::JSONRPCMessage; +#[cfg(windows)] +use codex_app_server_protocol::JSONRPCResponse; +#[cfg(windows)] +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +#[cfg(windows)] +use futures::SinkExt; +#[cfg(windows)] +use futures::StreamExt; +use pretty_assertions::assert_eq; +use tokio::io; +#[cfg(windows)] +use tokio::net::TcpListener; +#[cfg(windows)] +use tokio::net::TcpStream; +#[cfg(windows)] +use tokio::sync::oneshot; +#[cfg(windows)] +use tokio::time::Duration; +#[cfg(windows)] +use tokio::time::timeout; +#[cfg(windows)] +use tokio_tungstenite::WebSocketStream; +#[cfg(windows)] +use tokio_tungstenite::accept_async; +#[cfg(windows)] +use tokio_tungstenite::tungstenite::Message; + +use super::*; +use crate::client_api::ExecServerTransportParams; +#[cfg(windows)] +use crate::protocol::FS_READ_FILE_METHOD; +#[cfg(windows)] +use crate::protocol::FsReadFileParams; +#[cfg(windows)] +use crate::protocol::FsReadFileResponse; +#[cfg(windows)] +use crate::protocol::INITIALIZE_METHOD; +#[cfg(windows)] +use crate::protocol::INITIALIZED_METHOD; +#[cfg(windows)] +use crate::protocol::InitializeResponse; + +#[tokio::test] +async fn non_native_uri_is_rejected_before_connecting() { + let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new( + ExecServerTransportParams::websocket_url("not a websocket URL".to_string()), + )); + + let error = file_system + .read_file(&non_native_uri(), /*sandbox*/ None) + .await + .expect_err("non-native URI should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + +#[cfg(windows)] +#[tokio::test] +async fn remote_file_system_sends_explicit_windows_native_paths() { + let (websocket_url, captured_paths, server) = record_read_file_paths(2).await; + let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new( + ExecServerTransportParams::websocket_url(websocket_url), + )); + let paths = vec![ + ( + PathUri::parse("file:///C:/Users/Alice/src/main.rs").expect("valid drive URI"), + absolute_windows_path(r"C:\Users\Alice\src\main.rs"), + ), + ( + PathUri::parse("file://server/share/src/main.rs").expect("valid UNC URI"), + absolute_windows_path(r"\\server\share\src\main.rs"), + ), + ]; + let expected_paths = paths + .iter() + .map(|(_, expected_path)| expected_path.clone()) + .collect::>(); + + for (path, _) in paths { + assert_eq!( + file_system + .read_file(&path, /*sandbox*/ None) + .await + .expect("remote read should succeed"), + Vec::::new() + ); + } + + assert_eq!( + captured_paths.await.expect("captured paths"), + expected_paths + ); + server.await.expect("recording server should succeed"); +} + +fn non_native_uri() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/file.txt"; + #[cfg(windows)] + let uri = "file:///usr/local/file.txt"; + + PathUri::parse(uri).expect("valid non-native URI") +} + +#[cfg(windows)] +fn absolute_windows_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute Windows path") +} + +#[cfg(windows)] +async fn record_read_file_paths( + expected_requests: usize, +) -> ( + String, + oneshot::Receiver>, + tokio::task::JoinHandle<()>, +) { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let websocket_url = format!("ws://{}", listener.local_addr().expect("listener address")); + let (captured_paths_tx, captured_paths_rx) = oneshot::channel(); + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await.expect("listener should accept"); + let mut websocket = accept_async(stream) + .await + .expect("websocket handshake should succeed"); + complete_websocket_initialize(&mut websocket).await; + + let mut captured_paths = Vec::with_capacity(expected_requests); + for _ in 0..expected_requests { + let request = match read_jsonrpc_websocket(&mut websocket).await { + JSONRPCMessage::Request(request) if request.method == FS_READ_FILE_METHOD => { + request + } + other => panic!("expected fs/readFile request, got {other:?}"), + }; + let params: FsReadFileParams = + serde_json::from_value(request.params.expect("fs/readFile params should exist")) + .expect("fs/readFile params should deserialize"); + captured_paths.push(params.path); + write_jsonrpc_websocket( + &mut websocket, + JSONRPCMessage::Response(JSONRPCResponse { + id: request.id, + result: serde_json::to_value(FsReadFileResponse { + data_base64: String::new(), + }) + .expect("fs/readFile response should serialize"), + }), + ) + .await; + } + captured_paths_tx + .send(captured_paths) + .expect("captured paths receiver should stay open"); + }); + + (websocket_url, captured_paths_rx, server) +} + +#[cfg(windows)] +async fn complete_websocket_initialize(websocket: &mut WebSocketStream) { + let request = match read_jsonrpc_websocket(websocket).await { + JSONRPCMessage::Request(request) if request.method == INITIALIZE_METHOD => request, + other => panic!("expected initialize request, got {other:?}"), + }; + write_jsonrpc_websocket( + websocket, + JSONRPCMessage::Response(JSONRPCResponse { + id: request.id, + result: serde_json::to_value(InitializeResponse { + session_id: "session-1".to_string(), + }) + .expect("initialize response should serialize"), + }), + ) + .await; + + match read_jsonrpc_websocket(websocket).await { + JSONRPCMessage::Notification(notification) if notification.method == INITIALIZED_METHOD => { + } + other => panic!("expected initialized notification, got {other:?}"), + } +} + +#[cfg(windows)] +async fn read_jsonrpc_websocket(websocket: &mut WebSocketStream) -> JSONRPCMessage { + loop { + match timeout(Duration::from_secs(1), websocket.next()) + .await + .expect("json-rpc websocket read should not time out") + .expect("websocket should stay open") + .expect("websocket frame should read") + { + Message::Text(text) => { + return serde_json::from_str(text.as_ref()) + .expect("json-rpc text frame should parse"); + } + Message::Binary(bytes) => { + return serde_json::from_slice(bytes.as_ref()) + .expect("json-rpc binary frame should parse"); + } + Message::Ping(_) | Message::Pong(_) => {} + other => panic!("expected json-rpc websocket frame, got {other:?}"), + } + } +} + +#[cfg(windows)] +async fn write_jsonrpc_websocket( + websocket: &mut WebSocketStream, + message: JSONRPCMessage, +) { + let encoded = serde_json::to_string(&message).expect("json-rpc should serialize"); + websocket + .send(Message::Text(encoded.into())) + .await + .expect("json-rpc websocket frame should write"); +} diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 1460b2eb4e..0c107d852c 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -75,7 +75,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { let sandbox = require_platform_sandbox(sandbox)?; @@ -83,7 +83,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { .run_sandboxed( sandbox, FsHelperRequest::ReadFile(FsReadFileParams { - path: path.clone(), + path: path.to_abs_path()?, sandbox: None, }), ) @@ -260,3 +260,7 @@ fn map_sandbox_error(error: JSONRPCErrorError) -> io::Error { _ => io::Error::other(error.message), } } + +#[cfg(all(test, any(unix, windows)))] +#[path = "sandboxed_file_system_path_uri_tests.rs"] +mod path_uri_tests; diff --git a/codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs new file mode 100644 index 0000000000..eb1a0d5dc4 --- /dev/null +++ b/codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs @@ -0,0 +1,43 @@ +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; +use tokio::io; + +use super::*; + +#[tokio::test] +async fn sandboxed_file_system_rejects_non_native_uri_as_invalid_input() { + let runtime_paths = ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let file_system = SandboxedFileSystem::new(runtime_paths); + let sandbox = FileSystemSandboxContext::from_permission_profile( + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(Vec::new()), + NetworkSandboxPolicy::Restricted, + ), + ); + + let error = file_system + .read_file(&non_native_uri(), Some(&sandbox)) + .await + .expect_err("non-native URI should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + +fn non_native_uri() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/file.txt"; + #[cfg(windows)] + let uri = "file:///usr/local/file.txt"; + + match PathUri::parse(uri) { + Ok(uri) => uri, + Err(err) => panic!("valid non-native URI should parse: {err}"), + } +} diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index 18bc8bbfc6..eb36e9341c 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -53,9 +53,10 @@ impl FileSystemHandler { &self, params: FsReadFileParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let bytes = self .file_system - .read_file(¶ms.path, params.sandbox.as_ref()) + .read_file(&path, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsReadFileResponse { diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 7ae8b40734..18182b7d21 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -151,7 +151,6 @@ fn path_uri_join_and_parent_preserve_lexical_paths() -> Result<()> { joined_parent_traversal, PathUri::from_path(source_dir.join("../outside"))? ); - Ok(()) } @@ -169,7 +168,7 @@ async fn file_system_read_file_returns_bytes( std::fs::write(&file_path, "hello from trait")?; let contents = file_system - .read_file(&absolute_path(&file_path), /*sandbox*/ None) + .read_file(&PathUri::from_path(&file_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, b"hello from trait"); @@ -191,7 +190,7 @@ async fn file_system_read_file_text_returns_string( std::fs::write(&file_path, "hello from trait")?; let contents = file_system - .read_file_text(&absolute_path(&file_path), /*sandbox*/ None) + .read_file_text(&PathUri::from_path(&file_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, "hello from trait"); @@ -410,7 +409,7 @@ async fn file_system_sandboxed_read_allows_readable_root( let sandbox = read_only_sandbox(allowed_dir); let contents = file_system - .read_file(&absolute_path(&file_path), Some(&sandbox)) + .read_file(&PathUri::from_path(&file_path)?, Some(&sandbox)) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, b"sandboxed hello"); diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index 054d59a395..2e3617eb8a 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -22,6 +22,7 @@ use codex_exec_server::CreateDirectoryOptions; #[cfg(target_os = "linux")] use codex_exec_server::Environment; use codex_exec_server::RemoveOptions; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tempfile::TempDir; use test_case::test_case; @@ -324,7 +325,7 @@ async fn file_system_sandboxed_read_rejects_symlink_escape( let requested_path = allowed_dir.join("link").join("secret.txt"); let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .read_file(&absolute_path(&requested_path), Some(&sandbox)) + .read_file(&PathUri::from_path(&requested_path)?, Some(&sandbox)) .await { Ok(_) => anyhow::bail!("read should be blocked"), @@ -353,13 +354,14 @@ async fn file_system_sandboxed_read_rejects_symlink_parent_dotdot_escape( std::fs::write(&secret_path, "nope")?; symlink(&outside_dir, allowed_dir.join("link"))?; - let requested_path = absolute_path(allowed_dir.join("link").join("..").join("secret.txt")); + let requested_path = + PathUri::from_path(allowed_dir.join("link").join("..").join("secret.txt"))?; let sandbox = read_only_sandbox(allowed_dir); let error = match file_system.read_file(&requested_path, Some(&sandbox)).await { Ok(_) => anyhow::bail!("read should fail after path normalization"), Err(error) => error, }; - // AbsolutePathBuf normalizes `link/../secret.txt` to + // PathUri's native path constructor normalizes `link/../secret.txt` to // `allowed/secret.txt` before the request reaches the filesystem layer. // Depending on whether the platform/runtime resolves that normalized path // through a top-level symlink alias, the request can surface as either diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml index 41a1711cc0..cc2982f959 100644 --- a/codex-rs/ext/skills/Cargo.toml +++ b/codex-rs/ext/skills/Cargo.toml @@ -20,10 +20,10 @@ codex-exec-server = { workspace = true } codex-extension-api = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-string = { workspace = true } [dev-dependencies] async-trait = { workspace = true } -codex-utils-path-uri = { workspace = true } pretty_assertions = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index d16b2b9335..1ed1d11fa0 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -10,6 +10,7 @@ use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use crate::catalog::SkillAuthority; use crate::catalog::SkillCatalog; @@ -130,9 +131,15 @@ impl SkillProvider for ExecutorSkillProvider { "executor skill resource references unavailable environment `{environment_id}`" ))); }; + let resource_path = PathUri::from_abs_path(resource_path).map_err(|err| { + SkillProviderError::new(format!( + "failed to read executor skill resource {}: {err}", + request.resource.as_str() + )) + })?; let contents = environment .get_filesystem() - .read_file_text(resource_path, /*sandbox*/ None) + .read_file_text(&resource_path, /*sandbox*/ None) .await .map_err(|err| { SkillProviderError::new(format!( diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index 1df1f155e6..1163a4bb70 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -73,10 +73,10 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { - if path == &self.canonical_root.join("skill/SKILL.md") { + if path.to_abs_path()? == self.canonical_root.join("skill/SKILL.md") { Ok(SKILL_CONTENTS.as_bytes().to_vec()) } else { Err(io::Error::new(io::ErrorKind::NotFound, "not found")) diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index b9f9cd7173..286d38d2ce 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -143,14 +143,14 @@ pub trait ExecutorFileSystem: Send + Sync { async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult>; /// Reads a file and decodes it as UTF-8 text. async fn read_file_text( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { let bytes = self.read_file(path, sandbox).await?; diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index 9f9c6d0c1d..18ba488252 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -14,6 +14,7 @@ chrono = { workspace = true } codex-file-system = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } futures = { workspace = true, features = ["alloc"] } gix = { workspace = true } once_cell = { workspace = true } diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 7c0d59462c..aa581e49ae 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use codex_file_system::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::join_all; use schemars::JsonSchema; use serde::Deserialize; @@ -803,6 +804,7 @@ pub async fn resolve_root_git_project_for_trust( ) -> Option { let repo_root = get_git_repo_root_with_fs(fs, cwd).await?; let dot_git = repo_root.join(".git"); + let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?; if fs .get_metadata(&dot_git, /*sandbox*/ None) .await @@ -812,7 +814,10 @@ pub async fn resolve_root_git_project_for_trust( return Some(repo_root); } - let git_dir_s = fs.read_file_text(&dot_git, /*sandbox*/ None).await.ok()?; + let git_dir_s = fs + .read_file_text(&dot_git_uri, /*sandbox*/ None) + .await + .ok()?; let git_dir_rel = git_dir_s.trim().strip_prefix("gitdir:")?.trim(); if git_dir_rel.is_empty() { return None; diff --git a/codex-rs/utils/plugins/Cargo.toml b/codex-rs/utils/plugins/Cargo.toml index 983f4526ea..2c56ab3773 100644 --- a/codex-rs/utils/plugins/Cargo.toml +++ b/codex-rs/utils/plugins/Cargo.toml @@ -17,6 +17,7 @@ workspace = true codex-exec-server = { workspace = true } codex-login = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/utils/plugins/src/plugin_namespace.rs b/codex-rs/utils/plugins/src/plugin_namespace.rs index ea5caab52f..8604c87b30 100644 --- a/codex-rs/utils/plugins/src/plugin_namespace.rs +++ b/codex-rs/utils/plugins/src/plugin_namespace.rs @@ -2,6 +2,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::path::Path; use std::path::PathBuf; @@ -38,8 +39,9 @@ async fn plugin_manifest_name( } } let manifest_path = manifest_path?; + let manifest_path_uri = PathUri::from_abs_path(&manifest_path).ok()?; let contents = fs - .read_file_text(&manifest_path, /*sandbox*/ None) + .read_file_text(&manifest_path_uri, /*sandbox*/ None) .await .ok()?; let RawPluginManifestName { name: raw_name } = serde_json::from_str(&contents).ok()?;