diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 10bb76842292..30e88f655545 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4213,6 +4213,7 @@ dependencies = [ name = "codex-utils-path-uri" version = "0.0.0" dependencies = [ + "base64 0.22.1", "codex-utils-absolute-path", "pretty_assertions", "schemars 0.8.22", 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 c0d93bb158d8..c18300b1c176 100644 --- a/codex-rs/app-server/src/request_processors/fs_processor.rs +++ b/codex-rs/app-server/src/request_processors/fs_processor.rs @@ -65,7 +65,7 @@ impl FsRequestProcessor { &self, params: FsReadFileParams, ) -> Result { - let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; + let path = PathUri::from_abs_path(¶ms.path); let bytes = self .file_system()? .read_file(&path, /*sandbox*/ None) @@ -85,7 +85,7 @@ impl FsRequestProcessor { "fs/writeFile requires valid base64 dataBase64: {err}" )) })?; - let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; + let path = PathUri::from_abs_path(¶ms.path); self.file_system()? .write_file(&path, bytes, /*sandbox*/ None) .await @@ -97,7 +97,7 @@ impl FsRequestProcessor { &self, params: FsCreateDirectoryParams, ) -> Result { - let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; + let path = PathUri::from_abs_path(¶ms.path); self.file_system()? .create_directory( &path, @@ -115,7 +115,7 @@ impl FsRequestProcessor { &self, params: FsGetMetadataParams, ) -> Result { - let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; + let path = PathUri::from_abs_path(¶ms.path); let metadata = self .file_system()? .get_metadata(&path, /*sandbox*/ None) @@ -134,7 +134,7 @@ impl FsRequestProcessor { &self, params: FsReadDirectoryParams, ) -> Result { - let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; + let path = PathUri::from_abs_path(¶ms.path); let entries = self .file_system()? .read_directory(&path, /*sandbox*/ None) @@ -156,7 +156,7 @@ impl FsRequestProcessor { &self, params: FsRemoveParams, ) -> Result { - let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; + let path = PathUri::from_abs_path(¶ms.path); self.file_system()? .remove( &path, @@ -175,9 +175,8 @@ impl FsRequestProcessor { &self, params: FsCopyParams, ) -> Result { - let source_path = PathUri::from_abs_path(¶ms.source_path).map_err(map_fs_error)?; - let destination_path = - PathUri::from_abs_path(¶ms.destination_path).map_err(map_fs_error)?; + let source_path = PathUri::from_abs_path(¶ms.source_path); + let destination_path = PathUri::from_abs_path(¶ms.destination_path); self.file_system()? .copy( &source_path, diff --git a/codex-rs/apply-patch/src/invocation.rs b/codex-rs/apply-patch/src/invocation.rs index b66ee2291d16..8701750fbef2 100644 --- a/codex-rs/apply-patch/src/invocation.rs +++ b/codex-rs/apply-patch/src/invocation.rs @@ -186,17 +186,7 @@ pub async fn verify_apply_patch_args( ); } Hunk::DeleteFile { .. } => { - 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 path_uri = PathUri::from_abs_path(&path); let content = match fs.read_file_text(&path_uri, sandbox).await { Ok(content) => content, Err(e) => { diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 0800ed04b667..386f1e53469f 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -391,7 +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)?; + let path_uri = PathUri::from_abs_path(&path_abs); match hunk { Hunk::AddFile { contents, .. } => { let overwritten_content = @@ -556,7 +556,7 @@ async fn ensure_not_directory( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> io::Result<()> { - let path_uri = PathUri::from_abs_path(path)?; + let path_uri = PathUri::from_abs_path(path); let metadata = fs.get_metadata(&path_uri, sandbox).await?; if metadata.is_directory { return Err(io::Error::new( @@ -573,9 +573,7 @@ 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; - }; + let path_uri = PathUri::from_abs_path(path); match expected_content { Some(expected_content) => fs .read_file_text(&path_uri, sandbox) @@ -592,13 +590,7 @@ async fn read_optional_file_text_for_delta( exact: &mut bool, ) -> Option { note_existing_path_delta_support(path, fs, sandbox, exact).await; - let path_uri = match PathUri::from_abs_path(path) { - Ok(path_uri) => path_uri, - Err(_) => { - *exact = false; - return None; - } - }; + let path_uri = PathUri::from_abs_path(path); match fs.read_file_text(&path_uri, sandbox).await { Ok(content) => Some(content), Err(source) if source.kind() == io::ErrorKind::NotFound => None, @@ -615,10 +607,7 @@ async fn note_existing_path_delta_support( sandbox: Option<&FileSystemSandboxContext>, exact: &mut bool, ) { - let Ok(path_uri) = PathUri::from_abs_path(path) else { - *exact = false; - return; - }; + let path_uri = PathUri::from_abs_path(path); match fs.get_metadata(&path_uri, sandbox).await { Ok(metadata) if metadata.is_file && !metadata.is_symlink => {} Ok(_) => *exact = false, @@ -633,12 +622,12 @@ async fn write_file_with_missing_parent_retry( contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> anyhow::Result<()> { - let path_uri = PathUri::from_abs_path(path_abs)?; + let path_uri = PathUri::from_abs_path(path_abs); match fs.write_file(&path_uri, contents.clone(), sandbox).await { Ok(()) => Ok(()), Err(err) if err.kind() == io::ErrorKind::NotFound => { if let Some(parent_abs) = path_abs.parent() { - let parent_uri = PathUri::from_abs_path(&parent_abs)?; + let parent_uri = PathUri::from_abs_path(&parent_abs); fs.create_directory( &parent_uri, CreateDirectoryOptions { recursive: true }, @@ -676,7 +665,7 @@ async fn derive_new_contents_from_chunks( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> std::result::Result { - let path_uri = PathUri::from_abs_path(path_abs)?; + 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()), diff --git a/codex-rs/config/src/loader/layer_io.rs b/codex-rs/config/src/loader/layer_io.rs index 7cdb4f5359d8..947911a89314 100644 --- a/codex-rs/config/src/loader/layer_io.rs +++ b/codex-rs/config/src/loader/layer_io.rs @@ -107,7 +107,7 @@ pub(super) async fn read_config_from_path( log_missing_as_info: bool, strict_config: bool, ) -> io::Result> { - let path_uri = PathUri::from_abs_path(path)?; + 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) => { diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index d154f87e1d0a..6d496bc9c56c 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -473,7 +473,7 @@ async fn load_config_toml_for_required_layer( strict_config: bool, create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry, ) -> io::Result { - let toml_file_uri = PathUri::from_abs_path(toml_file)?; + 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(|| { @@ -569,7 +569,7 @@ 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)?; + let requirements_toml_file_uri = PathUri::from_abs_path(requirements_toml_file); match fs .read_file_text(&requirements_toml_file_uri, /*sandbox*/ None) .await @@ -1139,7 +1139,7 @@ async fn find_project_root( for ancestor in cwd.ancestors() { for marker in project_root_markers { let marker_path = ancestor.join(marker); - let marker_path_uri = PathUri::from_abs_path(&marker_path)?; + let marker_path_uri = PathUri::from_abs_path(&marker_path); if fs .get_metadata(&marker_path_uri, /*sandbox*/ None) .await @@ -1156,7 +1156,7 @@ async fn find_git_checkout_root( fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, ) -> Option { - let cwd_uri = PathUri::from_abs_path(cwd).ok()?; + let cwd_uri = PathUri::from_abs_path(cwd); let base = match fs.get_metadata(&cwd_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => cwd.clone(), _ => cwd.parent()?, @@ -1164,7 +1164,7 @@ async fn find_git_checkout_root( for dir in base.ancestors() { let dot_git = dir.join(".git"); - let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?; + let dot_git_uri = PathUri::from_abs_path(&dot_git); if fs .get_metadata(&dot_git_uri, /*sandbox*/ None) .await @@ -1217,7 +1217,7 @@ async fn load_project_layers( let mut startup_warnings = Vec::new(); for dir in dirs { let dot_codex_abs = dir.join(".codex"); - let dot_codex_uri = PathUri::from_abs_path(&dot_codex_abs)?; + let dot_codex_uri = PathUri::from_abs_path(&dot_codex_abs); if !fs .get_metadata(&dot_codex_uri, /*sandbox*/ None) .await @@ -1236,7 +1236,7 @@ async fn load_project_layers( continue; } let config_file = dot_codex_abs.join(CONFIG_TOML_FILE); - let config_file_uri = PathUri::from_abs_path(&config_file)?; + 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) { @@ -1340,7 +1340,7 @@ 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 hooks_config_file_uri = PathUri::from_abs_path(&hooks_config_file); let root_config = match fs .read_file_text(&hooks_config_file_uri, /*sandbox*/ None) .await diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 88c5b16522db..543c307c3548 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -21,7 +21,7 @@ impl ExecutorFileSystem for TestFileSystem { Box::pin(async move { let path = path.to_abs_path()?; let canonicalized = path.canonicalize()?; - PathUri::from_abs_path(&canonicalized) + Ok(PathUri::from_abs_path(&canonicalized)) }) } diff --git a/codex-rs/core-plugins/src/provider.rs b/codex-rs/core-plugins/src/provider.rs index 2ba1ecca6d83..3e3e0c32ffca 100644 --- a/codex-rs/core-plugins/src/provider.rs +++ b/codex-rs/core-plugins/src/provider.rs @@ -137,17 +137,8 @@ async fn resolve_plugin_root( file_system: &dyn ExecutorFileSystem, ) -> Result, ExecutorPluginProviderError> { let root_id = &selected_root.id; - let CapabilityRootLocation::Environment { - environment_id, - path, - } = &selected_root.location; - let root_uri = PathUri::from_abs_path(&plugin_root).map_err(|err| { - ExecutorPluginProviderError::InvalidRootPath { - root_id: root_id.clone(), - path: path.clone(), - message: err.to_string(), - } - })?; + let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; + let root_uri = PathUri::from_abs_path(&plugin_root); let root_metadata = file_system .get_metadata(&root_uri, /*sandbox*/ None) .await @@ -166,13 +157,7 @@ async fn resolve_plugin_root( let mut manifest_path = None; for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS { let candidate = plugin_root.join(relative_path); - let candidate_uri = PathUri::from_abs_path(&candidate).map_err(|err| { - ExecutorPluginProviderError::InvalidRootPath { - root_id: root_id.clone(), - path: candidate.as_path().to_string_lossy().into_owned(), - message: err.to_string(), - } - })?; + let candidate_uri = PathUri::from_abs_path(&candidate); match file_system .get_metadata(&candidate_uri, /*sandbox*/ None) .await diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index 03bb0a6dddd3..e7d1fd374941 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -76,20 +76,7 @@ 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)); - // 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; - } - }; + let path = PathUri::from_abs_path(&skill.path_to_skills_md); match fs.read_file_text(&path, /*sandbox*/ None).await { Ok(contents) => { emit_skill_injected_metric(otel, skill, "ok"); diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 813f13018d61..3b59e908d6d6 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -375,16 +375,7 @@ async fn repo_agents_skill_roots( let mut roots = Vec::new(); for dir in dirs { let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME); - let agents_skills_uri = match PathUri::from_abs_path(&agents_skills) { - Ok(path) => path, - Err(err) => { - tracing::warn!( - "failed to convert repo skills root {} to URI: {err:#}", - agents_skills.display() - ); - continue; - } - }; + let agents_skills_uri = PathUri::from_abs_path(&agents_skills); match fs.get_metadata(&agents_skills_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => roots.push(SkillRoot { path: agents_skills, @@ -440,16 +431,7 @@ async fn find_project_root( for ancestor in cwd.ancestors() { for marker in project_root_markers { let marker_path = ancestor.join(marker); - let marker_path_uri = match PathUri::from_abs_path(&marker_path) { - Ok(path) => path, - Err(err) => { - tracing::warn!( - "failed to convert project root marker {} to URI: {err:#}", - marker_path.display() - ); - continue; - } - }; + let marker_path_uri = PathUri::from_abs_path(&marker_path); match fs.get_metadata(&marker_path_uri, /*sandbox*/ None).await { Ok(_) => return ancestor, Err(err) if err.kind() == io::ErrorKind::NotFound => {} @@ -496,9 +478,7 @@ async fn canonicalize_for_skill_identity( fs: &dyn ExecutorFileSystem, path: &AbsolutePathBuf, ) -> AbsolutePathBuf { - let Ok(path_uri) = PathUri::from_abs_path(path) else { - return path.clone(); - }; + let path_uri = PathUri::from_abs_path(path); fs.canonicalize(&path_uri, /*sandbox*/ None) .await .and_then(|path| path.to_abs_path()) @@ -519,16 +499,7 @@ async fn discover_skills_under_root( None => None, }; - let root_uri = match PathUri::from_abs_path(&root) { - Ok(path) => path, - Err(err) => { - tracing::warn!( - "failed to convert skills root {} to URI: {err:#}", - root.display() - ); - return; - } - }; + let root_uri = PathUri::from_abs_path(&root); match fs.get_metadata(&root_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => {} Ok(_) => return, @@ -571,16 +542,7 @@ async fn discover_skills_under_root( let mut truncated_by_dir_limit = false; while let Some((dir, depth)) = queue.pop_front() { - let dir_uri = match PathUri::from_abs_path(&dir) { - Ok(path) => path, - Err(e) => { - tracing::warn!( - "failed to convert skills dir {} to URI: {e:#}", - dir.display() - ); - continue; - } - }; + let dir_uri = PathUri::from_abs_path(&dir); let entries = match fs.read_directory(&dir_uri, /*sandbox*/ None).await { Ok(entries) => entries, Err(e) => { @@ -596,16 +558,7 @@ async fn discover_skills_under_root( } let path = dir.join(&file_name); - let path_uri = match PathUri::from_abs_path(&path) { - Ok(path) => path, - Err(e) => { - tracing::warn!( - "failed to convert skills path {} to URI: {e:#}", - path.display() - ); - continue; - } - }; + let path_uri = PathUri::from_abs_path(&path); let metadata = match fs.get_metadata(&path_uri, /*sandbox*/ None).await { Ok(metadata) => metadata, Err(e) => { @@ -690,7 +643,7 @@ 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 path_uri = PathUri::from_abs_path(path); let contents = fs .read_file_text(&path_uri, /*sandbox*/ None) .await @@ -786,17 +739,7 @@ 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(); - } - }; + let metadata_path_uri = PathUri::from_abs_path(&metadata_path); match fs.get_metadata(&metadata_path_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_file => {} Ok(_) => return LoadedSkillMetadata::default(), diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index 5569655adf7d..3abb30c96d0c 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -153,7 +153,7 @@ impl HostLoadedSkills { .outcome .file_system_for_skill(skill) .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - let path = PathUri::from_abs_path(&skill.path_to_skills_md)?; + 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/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 49ac6fc723cc..d63342bdded4 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -113,7 +113,7 @@ async fn read_agents_md( break; } - let path_uri = PathUri::from_abs_path(&p)?; + let path_uri = PathUri::from_abs_path(&p); match fs.get_metadata(&path_uri, /*sandbox*/ None).await { Ok(metadata) if !metadata.is_file => continue, Ok(_) => {} @@ -194,7 +194,7 @@ async fn agents_md_paths( for ancestor in dir.ancestors() { for marker in &project_root_markers { let marker_path = ancestor.join(marker); - let marker_path_uri = PathUri::from_abs_path(&marker_path)?; + let marker_path_uri = PathUri::from_abs_path(&marker_path); let marker_exists = match fs.get_metadata(&marker_path_uri, /*sandbox*/ None).await { Ok(_) => true, @@ -236,7 +236,7 @@ async fn agents_md_paths( for d in search_dirs { for name in &candidate_filenames { let candidate = d.join(name); - let candidate_uri = PathUri::from_abs_path(&candidate)?; + let candidate_uri = PathUri::from_abs_path(&candidate); match fs.get_metadata(&candidate_uri, /*sandbox*/ None).await { Ok(md) if md.is_file => { found.push(candidate); diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index e0a046a7ea64..8d0987ca4dcd 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -268,7 +268,6 @@ fn resolved_local_environments( cwd, /*shell*/ None, ) - .expect("local cwd URI") }) .collect(), } diff --git a/codex-rs/core/src/config/agent_roles.rs b/codex-rs/core/src/config/agent_roles.rs index dfcec2eac817..483cd3a98dc8 100644 --- a/codex-rs/core/src/config/agent_roles.rs +++ b/codex-rs/core/src/config/agent_roles.rs @@ -320,7 +320,7 @@ async fn read_resolved_agent_role_file( path: &AbsolutePathBuf, role_name_hint: Option<&str>, ) -> std::io::Result { - let path_uri = PathUri::from_abs_path(path)?; + 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( @@ -393,7 +393,7 @@ async fn validate_agent_role_config_file( return Ok(()); }; - let config_file_uri = PathUri::from_abs_path(config_file)?; + let config_file_uri = PathUri::from_abs_path(config_file); let metadata = fs .get_metadata(&config_file_uri, /*sandbox*/ None) .await @@ -525,7 +525,7 @@ async fn collect_agent_role_files( let mut files = Vec::new(); let mut dirs = vec![dir.clone()]; while let Some(dir) = dirs.pop() { - let dir_uri = PathUri::from_abs_path(&dir)?; + let dir_uri = PathUri::from_abs_path(&dir); let entries = match fs.read_directory(&dir_uri, /*sandbox*/ None).await { Ok(entries) => entries, Err(err) if err.kind() == ErrorKind::NotFound => continue, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index a96f005cb600..9fe65a836bc9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -3696,7 +3696,7 @@ impl Config { return Ok(None); }; - let path_uri = PathUri::from_abs_path(path)?; + let path_uri = PathUri::from_abs_path(path); let contents = fs .read_file_text(&path_uri, /*sandbox*/ None) .await diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 4e590d3ecf6c..67cd1ccac69c 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -101,7 +101,7 @@ pub(crate) async fn resolve_environment_selections( environment, selected_environment.cwd.clone(), shell, - )?); + )); } Ok(ResolvedTurnEnvironments { turn_environments }) } @@ -270,15 +270,12 @@ url = "ws://127.0.0.1:8765" .expect("remote environment"), ); let remote = ResolvedTurnEnvironments { - turn_environments: vec![ - TurnEnvironment::new( - REMOTE_ENVIRONMENT_ID.to_string(), - remote_environment.clone(), - cwd.clone(), - /*shell*/ None, - ) - .expect("remote cwd URI"), - ], + turn_environments: vec![TurnEnvironment::new( + REMOTE_ENVIRONMENT_ID.to_string(), + remote_environment.clone(), + cwd.clone(), + /*shell*/ None, + )], }; let multiple = ResolvedTurnEnvironments { turn_environments: vec![ @@ -288,8 +285,7 @@ url = "ws://127.0.0.1:8765" remote_environment, cwd.clone(), /*shell*/ None, - ) - .expect("remote cwd URI"), + ), ], }; diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 0b3caac36158..9468534d8fd3 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -373,14 +373,8 @@ pub fn build_exec_request( "command args are empty", )) })?; - let cwd = PathUri::from_abs_path(&cwd).map_err(|_| { - CodexErr::InvalidRequest("command cwd cannot be represented as a file URI".to_string()) - })?; - let sandbox_policy_cwd_uri = PathUri::from_abs_path(sandbox_cwd).map_err(|_| { - CodexErr::InvalidRequest( - "sandbox policy cwd cannot be represented as a file URI".to_string(), - ) - })?; + let cwd = PathUri::from_abs_path(&cwd); + let sandbox_policy_cwd_uri = PathUri::from_abs_path(sandbox_cwd); let manager = SandboxManager::new(); let command = SandboxCommand { diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 8068524d93a9..a647eee8b25b 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4035,15 +4035,12 @@ fn turn_environments_for_tests( cwd: &codex_utils_absolute_path::AbsolutePathBuf, ) -> crate::environment_selection::ResolvedTurnEnvironments { crate::environment_selection::ResolvedTurnEnvironments { - turn_environments: vec![ - TurnEnvironment::new( - codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - Arc::clone(environment), - cwd.clone(), - /*shell*/ None, - ) - .expect("turn environment"), - ], + turn_environments: vec![TurnEnvironment::new( + codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + Arc::clone(environment), + cwd.clone(), + /*shell*/ None, + )], } } @@ -5665,8 +5662,7 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir current_environment.environment, environment_cwd.clone(), current_environment.shell, - ) - .expect("environment cwd URI"); + ); let call_id = "call-1".to_string(); let handler = RequestPermissionsHandler; @@ -6255,15 +6251,15 @@ async fn primary_environment_uses_first_turn_environment() { let first_environment = turn_context.environments.turn_environments[0].clone(); #[allow(deprecated)] let second_cwd = turn_context.cwd.join("second"); - turn_context.environments.turn_environments.push( - TurnEnvironment::new( + turn_context + .environments + .turn_environments + .push(TurnEnvironment::new( "second".to_string(), Arc::clone(&first_environment.environment), second_cwd.clone(), /*shell*/ None, - ) - .expect("turn environment"), - ); + )); assert_eq!( turn_context diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 99d1482c7fc2..655ce1a39389 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -56,19 +56,15 @@ impl TurnEnvironment { environment: Arc, cwd: AbsolutePathBuf, shell: Option, - ) -> CodexResult { - let cwd_uri = PathUri::from_abs_path(&cwd).map_err(|_| { - CodexErr::InvalidRequest( - "turn environment cwd cannot be represented as a file URI".to_string(), - ) - })?; - Ok(Self { + ) -> Self { + let cwd_uri = PathUri::from_abs_path(&cwd); + Self { environment_id, environment, cwd, cwd_uri, shell, - }) + } } pub(crate) fn cwd(&self) -> &AbsolutePathBuf { diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index fc1c4321824d..a719e9acd9f6 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -526,12 +526,7 @@ pub(crate) async fn intercept_apply_patch( call_id: &str, tool_name: &str, ) -> Result, FunctionCallError> { - let sandbox_cwd = PathUri::from_abs_path(cwd).map_err(|_| { - FunctionCallError::RespondToModel( - "unable to prepare filesystem sandbox: working directory cannot be represented as a file URI" - .to_string(), - ) - })?; + let sandbox_cwd = PathUri::from_abs_path(cwd); let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, &sandbox_cwd); match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd, fs, Some(&sandbox)) diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 6a66a43a8b8e..dc52e7e336c7 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -150,12 +150,7 @@ impl ViewImageHandler { turn_environment.cwd_uri(), ); 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 path_uri = PathUri::from_abs_path(&abs_path); let metadata = fs .get_metadata(&path_uri, Some(&sandbox)) @@ -295,8 +290,7 @@ mod tests { current.environment, cwd, current.shell, - ) - .expect("image cwd URI"); + ); } #[test] diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index f0b0f8ca0882..7e97e4ad5144 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -240,11 +240,7 @@ impl ToolOrchestrator { let use_legacy_landlock = turn_ctx.features.use_legacy_landlock(); #[allow(deprecated)] let sandbox_cwd = tool.sandbox_cwd(req).unwrap_or(&turn_ctx.cwd); - let sandbox_policy_cwd = PathUri::from_abs_path(sandbox_cwd).map_err(|_| { - ToolError::Codex(CodexErr::InvalidRequest( - "sandbox policy cwd cannot be represented as a file URI".to_string(), - )) - })?; + let sandbox_policy_cwd = PathUri::from_abs_path(sandbox_cwd); let workspace_roots = turn_ctx.config.effective_workspace_roots(); let initial_attempt = SandboxAttempt { sandbox: initial_sandbox, diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index e3b1cc90ac2a..044002f44653 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -22,7 +22,6 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context:: std::env::temp_dir().abs(), /*shell*/ None, ) - .expect("turn environment") } #[test] @@ -208,7 +207,7 @@ async fn file_system_sandbox_context_uses_active_attempt() { NetworkSandboxPolicy::Restricted, ); let manager = SandboxManager::new(); - let sandbox_policy_cwd = PathUri::from_abs_path(&path).expect("path URI"); + let sandbox_policy_cwd = PathUri::from_abs_path(&path); let attempt = SandboxAttempt { sandbox: SandboxType::MacosSeatbelt, permissions: &permissions, @@ -237,7 +236,7 @@ async fn file_system_sandbox_context_uses_active_attempt() { assert_eq!(sandbox.permissions, expected_permissions); assert_eq!( sandbox.cwd, - Some(codex_utils_path_uri::PathUri::from_abs_path(&path).expect("path URI")) + Some(codex_utils_path_uri::PathUri::from_abs_path(&path)) ); assert_eq!( sandbox.windows_sandbox_level, @@ -266,7 +265,7 @@ async fn no_sandbox_attempt_has_no_file_system_context() { }; let permissions = PermissionProfile::Disabled; let manager = SandboxManager::new(); - let sandbox_policy_cwd = PathUri::from_abs_path(&path).expect("path URI"); + let sandbox_policy_cwd = PathUri::from_abs_path(&path); let attempt = SandboxAttempt { sandbox: SandboxType::None, permissions: &permissions, diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 241ce4a5432d..7940f397eec3 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -21,7 +21,6 @@ use codex_network_proxy::PROXY_ENV_KEYS; use codex_network_proxy::PROXY_GIT_SSH_COMMAND_ENV_KEY; use codex_network_proxy::is_managed_mitm_ca_trust_bundle_path; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::error::CodexErr; use codex_protocol::models::AdditionalPermissionProfile; use codex_sandboxing::SandboxCommand; use codex_sandboxing::SandboxType; @@ -36,8 +35,7 @@ pub(crate) mod shell; pub(crate) mod unified_exec; /// Shared helper to construct sandbox transform inputs from a tokenized command line and native -/// working directory. Validates that at least a program is present and that the working directory -/// has a file URI representation. +/// working directory. Validates that at least a program is present. pub(crate) fn build_sandbox_command( command: &[String], cwd: &AbsolutePathBuf, @@ -47,11 +45,7 @@ pub(crate) fn build_sandbox_command( let (program, args) = command .split_first() .ok_or_else(|| ToolError::Rejected("command args are empty".to_string()))?; - let cwd = PathUri::from_abs_path(cwd).map_err(|_| { - ToolError::Codex(CodexErr::InvalidRequest( - "command cwd cannot be represented as a file URI".to_string(), - )) - })?; + let cwd = PathUri::from_abs_path(cwd); Ok(SandboxCommand { program: program.clone().into(), args: args.to_vec(), diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index 33f29991cf17..4a88d014df3f 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -102,8 +102,8 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: /*additional_permissions*/ None, ) .expect("build sandbox command"); - assert_eq!(command.cwd, PathUri::from_abs_path(&command_cwd)?); - let sandbox_policy_cwd = PathUri::from_abs_path(&native_sandbox_policy_cwd)?; + assert_eq!(command.cwd, PathUri::from_abs_path(&command_cwd)); + let sandbox_policy_cwd = PathUri::from_abs_path(&native_sandbox_policy_cwd); let options = ExecOptions { expiration: ExecExpiration::DefaultTimeout, capture_policy: ExecCapturePolicy::ShellTool, diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 7e854d8ef4c5..ea53e9a55c89 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -969,15 +969,8 @@ impl CoreShellCommandExecutor { self.windows_sandbox_level, self.network.is_some(), ); - let cwd = PathUri::from_abs_path(workdir).map_err(|_| { - CodexErr::InvalidRequest("command cwd cannot be represented as a file URI".to_string()) - })?; - let sandbox_policy_cwd = - PathUri::from_abs_path(&self.sandbox_policy_cwd).map_err(|_| { - CodexErr::InvalidRequest( - "sandbox policy cwd cannot be represented as a file URI".to_string(), - ) - })?; + let cwd = PathUri::from_abs_path(workdir); + let sandbox_policy_cwd = PathUri::from_abs_path(&self.sandbox_policy_cwd); let command = SandboxCommand { program: program.clone().into(), args: args.to_vec(), diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 8e28c1c36004..7a1e5d8aa936 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -586,8 +586,7 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava ), remote_cwd, /*shell*/ None, - ) - .expect("turn environment"), + ), ); }) .await; diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 1582b44d7dd2..cd7541bb1574 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -1005,7 +1005,7 @@ impl TestCodexHarness { } pub async fn remove_abs_path(&self, path: &AbsolutePathBuf) -> Result<()> { - let path_uri = PathUri::from_abs_path(path)?; + let path_uri = PathUri::from_abs_path(path); self.test .fs() .remove( @@ -1021,7 +1021,7 @@ impl TestCodexHarness { } pub async fn abs_path_exists(&self, path: &AbsolutePathBuf) -> Result { - let path_uri = PathUri::from_abs_path(path)?; + let path_uri = PathUri::from_abs_path(path); match self .test .fs() diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 2f18425c2a48..3ec2301aaf2e 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -583,7 +583,7 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result test.fs() .remove( - &PathUri::from_abs_path(&remote_cwd)?, + &PathUri::from_abs_path(&remote_cwd), RemoveOptions { recursive: true, force: true, @@ -1092,7 +1092,7 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> { let symlink_exists = file_system .get_metadata( - &PathUri::from_abs_path(&absolute_path(symlink_path))?, + &PathUri::from_abs_path(&absolute_path(symlink_path)), /*sandbox*/ None, ) .await diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index bc100b94227c..688c421dd3a9 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -882,7 +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 path = codex_utils_path_uri::PathUri::from_abs_path(&path); 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_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 9c88c6b8859b..a99a9496babf 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -145,11 +145,7 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result PathUri { - PathUri::from_abs_path(&absolute_test_path(name)).expect("path URI") + PathUri::from_abs_path(&absolute_test_path(name)) } } diff --git a/codex-rs/ext/image-generation/src/tool.rs b/codex-rs/ext/image-generation/src/tool.rs index 3712ebbb0ed1..66bb9ec71c79 100644 --- a/codex-rs/ext/image-generation/src/tool.rs +++ b/codex-rs/ext/image-generation/src/tool.rs @@ -341,12 +341,7 @@ async fn image_url( path: &AbsolutePathBuf, environment: &ToolEnvironment, ) -> Result { - let path_uri = PathUri::from_abs_path(path).map_err(|error| { - FunctionCallError::RespondToModel(format!( - "unable to read referenced image at `{}`: {error}", - path.display() - )) - })?; + let path_uri = PathUri::from_abs_path(path); let bytes = environment .file_system .read_file(&path_uri, Some(&environment.file_system_sandbox_context)) diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index 1ed1d11fa0dc..2d26ae8bed00 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -131,12 +131,7 @@ 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 resource_path = PathUri::from_abs_path(resource_path); let contents = environment .get_filesystem() .read_file_text(&resource_path, /*sandbox*/ None) 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 b57f1db6bb11..02443351fd90 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -38,10 +38,10 @@ impl SyntheticFileSystem { async fn canonicalize(&self, path: &PathUri) -> io::Result { let path = path.to_abs_path()?; if path == self.alias_root { - return PathUri::from_abs_path(&self.canonical_root); + return Ok(PathUri::from_abs_path(&self.canonical_root)); } self.metadata(&path)?; - PathUri::from_abs_path(&path) + Ok(PathUri::from_abs_path(&path)) } async fn read_file(&self, path: &PathUri) -> io::Result> { diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index beff18b519ca..48d229df8db5 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -47,7 +47,7 @@ pub async fn get_git_repo_root_with_fs( fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, ) -> Option { - let cwd_uri = PathUri::from_abs_path(cwd).ok()?; + let cwd_uri = PathUri::from_abs_path(cwd); let base = match fs.get_metadata(&cwd_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => cwd.clone(), _ => cwd.parent()?, @@ -805,7 +805,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()?; + let dot_git_uri = PathUri::from_abs_path(&dot_git); if fs .get_metadata(&dot_git_uri, /*sandbox*/ None) .await @@ -859,7 +859,7 @@ async fn find_ancestor_git_entry_with_fs( ) -> Option<(AbsolutePathBuf, AbsolutePathBuf)> { for dir in base_dir.ancestors() { let dot_git = dir.join(".git"); - let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?; + let dot_git_uri = PathUri::from_abs_path(&dot_git); if fs .get_metadata(&dot_git_uri, /*sandbox*/ None) .await diff --git a/codex-rs/sandboxing/src/manager_tests.rs b/codex-rs/sandboxing/src/manager_tests.rs index 2cbcc483f623..d76a1fd325d0 100644 --- a/codex-rs/sandboxing/src/manager_tests.rs +++ b/codex-rs/sandboxing/src/manager_tests.rs @@ -75,7 +75,7 @@ fn restricted_file_system_uses_platform_sandbox_without_managed_network() { fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); - let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); + let cwd_uri = PathUri::from_abs_path(&cwd); let permissions = PermissionProfile::from_runtime_permissions( &FileSystemSandboxPolicy::unrestricted(), NetworkSandboxPolicy::Restricted, @@ -117,7 +117,7 @@ fn transform_preserves_unrestricted_file_system_policy_for_restricted_network() fn transform_additional_permissions_enable_network_for_external_sandbox() { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); - let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); + let cwd_uri = PathUri::from_abs_path(&cwd); let permissions = PermissionProfile::External { network: NetworkSandboxPolicy::Restricted, }; @@ -171,7 +171,7 @@ fn transform_additional_permissions_enable_network_for_external_sandbox() { fn transform_additional_permissions_preserves_denied_entries() { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); - let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); + let cwd_uri = PathUri::from_abs_path(&cwd); let temp_dir = TempDir::new().expect("create temp dir"); let workspace_root = AbsolutePathBuf::from_absolute_path( canonicalize(temp_dir.path()).expect("canonicalize temp dir"), @@ -297,7 +297,7 @@ fn transform_linux_seccomp_request( ) -> super::SandboxExecRequest { let manager = SandboxManager::new(); let cwd = AbsolutePathBuf::current_dir().expect("current dir"); - let cwd_uri = PathUri::from_abs_path(&cwd).expect("cwd URI"); + let cwd_uri = PathUri::from_abs_path(&cwd); let permissions = PermissionProfile::Disabled; manager .transform(SandboxTransformRequest { diff --git a/codex-rs/utils/path-uri/Cargo.toml b/codex-rs/utils/path-uri/Cargo.toml index 1cf70708e743..5bb6328139c7 100644 --- a/codex-rs/utils/path-uri/Cargo.toml +++ b/codex-rs/utils/path-uri/Cargo.toml @@ -8,6 +8,7 @@ license.workspace = true workspace = true [dependencies] +base64 = { workspace = true } codex-utils-absolute-path = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index 444af1ea28c3..8c36a9d4f662 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -2,6 +2,7 @@ //! //! See [`PathUri`] for scheme, normalization, and serialization behavior. +use base64::Engine; use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; use serde::Deserialize; @@ -17,13 +18,15 @@ use ts_rs::TS; use url::Url; pub const FILE_SCHEME: &str = "file"; +const BAD_PATH_URI_PREFIX: &str = "file:///%00/bad/path/"; /// An immutable, cross-platform representation of a `file:` URI. /// /// Only the `file:` scheme is currently accepted. Construction validates the /// URL, and the URI cannot be mutated after construction. [`Self::basename`], /// [`Self::parent`], and [`Self::join`] operate on URI path segments without -/// interpreting them using the operating system running Codex. +/// interpreting them using the operating system running Codex. Fallback URIs +/// created by [`Self::from_abs_path`] are opaque to these lexical operations. /// /// `file:` paths retain their URI spelling so they can be parsed independently /// of the current host. In particular, `/C:/src` remains ambiguous between a @@ -57,27 +60,53 @@ impl PathUri { /// Converts an absolute path on the current host to a `file:` URI. /// - /// On Windows, paths without a URI representation, including `\\.\` device - /// paths and generic `\\?\` verbatim namespaces, are reported as invalid - /// input. - pub fn from_abs_path(path: &AbsolutePathBuf) -> io::Result { - let url = Url::from_file_path(path.as_path()).map_err(|()| { - io::Error::new( - io::ErrorKind::InvalidInput, - PathUriParseError::InvalidFileUriPath, - ) - })?; - Self::try_from(url).map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err)) + /// 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 + /// 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 + /// real path on Unix or Windows. + 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) + { + return uri; + } + + #[cfg(unix)] + let encoded_path = { + use std::os::unix::ffi::OsStrExt; + base64::engine::general_purpose::URL_SAFE_NO_PAD + .encode(path.as_path().as_os_str().as_bytes()) + }; + #[cfg(windows)] + let encoded_path = { + use std::os::windows::ffi::OsStrExt; + let path_bytes = path + .as_path() + .as_os_str() + .encode_wide() + .flat_map(u16::to_le_bytes) + .collect::>(); + base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(path_bytes) + }; + let Ok(uri) = Self::parse(&format!("{BAD_PATH_URI_PREFIX}{encoded_path}")) else { + unreachable!("URL-safe base64 always produces a valid fallback path URI"); + }; + uri } /// Converts a path on the current host to a `file:` URI. /// - /// Relative paths and paths without a URI representation are reported as - /// invalid input. + /// Relative paths are reported as invalid input. Absolute paths without a + /// valid URI representation use the fallback documented on + /// [`Self::from_abs_path`]. pub fn from_path(path: impl AsRef) -> io::Result { let path = AbsolutePathBuf::from_absolute_path_checked(path) .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; - Self::from_abs_path(&path) + Ok(Self::from_abs_path(&path)) } /// Returns the percent-encoded URI path. @@ -88,20 +117,26 @@ impl PathUri { self.0.path() } - /// Returns the decoded final URI path segment, or `None` for the URI root. + /// Returns the decoded final URI path segment, or `None` for the URI root + /// or an opaque fallback URI created by [`Self::from_abs_path`]. /// /// If the segment contains non-UTF-8 encoded bytes, its percent-encoded /// spelling is returned instead. pub fn basename(&self) -> Option { + if decode_bad_path_uri(&self.0).is_some() { + return None; + } + self.0 .path_segments()? .rfind(|segment| !segment.is_empty()) .map(decode_uri_path) } - /// Returns the parent URI, or `None` for the URI root. + /// Returns the parent URI, or `None` for the URI root or an opaque fallback + /// URI created by [`Self::from_abs_path`]. pub fn parent(&self) -> Option { - if self.encoded_path() == "/" { + if self.encoded_path() == "/" || decode_bad_path_uri(&self.0).is_some() { return None; } @@ -122,6 +157,8 @@ impl PathUri { /// without escaping the URI root. Literal `%`, `?`, and `#` characters are /// percent-encoded as filename text. Paths containing a null character are /// rejected because they cannot be safely converted to native paths. + /// Opaque fallback URIs created by [`Self::from_abs_path`] reject non-empty + /// joins. pub fn join(&self, path: &str) -> Result { if path.starts_with('/') { return Err(PathUriParseError::JoinPathMustBeRelative(path.to_string())); @@ -132,6 +169,9 @@ impl PathUri { if path.is_empty() { return Ok(self.clone()); } + if decode_bad_path_uri(&self.0).is_some() { + return Err(PathUriParseError::InvalidFileUriPath); + } let mut url = self.0.clone(); { @@ -157,13 +197,46 @@ impl PathUri { /// Converts this file URI to a path using the current host's path rules. /// /// Conversion should succeed when the URI was created from an - /// [`AbsolutePathBuf`] on the current host. It may fail when the URI came - /// from a different operating system and its `file:` URI form cannot be - /// represented using the current host's path rules, such as a UNC authority - /// on POSIX or a POSIX root on Windows. Because a `file:` URI does not record - /// its source operating system, callers should only use this method when the - /// URI is known to identify a path on the current host. + /// [`AbsolutePathBuf`] on the current host, including fallback URIs created + /// by [`Self::from_abs_path`]. It may fail when the URI came from a different + /// operating system and its `file:` URI form cannot be represented using + /// the current host's path rules, such as a UNC authority on POSIX or a + /// POSIX root on Windows. Because a `file:` URI does not record its source + /// operating system, callers should only use this method when the URI is + /// known to identify a path on the current host. pub fn to_abs_path(&self) -> io::Result { + if let Some(path_bytes) = decode_bad_path_uri(&self.0) { + #[cfg(unix)] + let decoded_path = { + use std::os::unix::ffi::OsStringExt; + Some(std::path::PathBuf::from(std::ffi::OsString::from_vec( + path_bytes, + ))) + }; + #[cfg(windows)] + let decoded_path = { + use std::os::windows::ffi::OsStringExt; + path_bytes.len().is_multiple_of(2).then(|| { + let path_wide = path_bytes + .chunks_exact(2) + .map(|bytes| u16::from_le_bytes([bytes[0], bytes[1]])) + .collect::>(); + std::path::PathBuf::from(std::ffi::OsString::from_wide(&path_wide)) + }) + }; + if let Some(decoded_path) = decoded_path + && let Ok(path) = AbsolutePathBuf::from_absolute_path_checked(decoded_path) + && Self::from_abs_path(&path).eq(self) + { + return Ok(path); + } + + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + PathUriParseError::InvalidFileUriPath, + )); + } + let path = self.0.to_file_path().map_err(|()| { io::Error::new( io::ErrorKind::InvalidInput, @@ -232,7 +305,7 @@ impl<'de> Deserialize<'de> for PathUri { .map_or_else(|| path_error.to_string(), |error| error.to_string()), ) })?; - Self::from_abs_path(&path).map_err(serde::de::Error::custom) + Ok(Self::from_abs_path(&path)) } } @@ -290,6 +363,20 @@ fn decode_uri_path(path: &str) -> String { .unwrap_or_else(|_| path.to_string()) } +/// Returns the original platform path bytes from a canonical bad-path URI. +fn decode_bad_path_uri(url: &Url) -> Option> { + let encoded_path = url.as_str().strip_prefix(BAD_PATH_URI_PREFIX)?; + if encoded_path.is_empty() || encoded_path.contains('/') { + return None; + } + + let path_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(encoded_path) + .ok()?; + (base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(&path_bytes) == encoded_path) + .then_some(path_bytes) +} + /// Rejects URI metadata that has no defined meaning for `file:` URIs. fn validate_common_known_uri(url: &Url) -> Result<(), PathUriParseError> { if !url.username().is_empty() || url.password().is_some() { @@ -312,7 +399,9 @@ fn validate_file_url(url: &Url) -> Result<(), PathUriParseError> { validate_common_known_uri(url)?; // `Url` accepts `%00`, but native path APIs use null as a terminator and // `Url::to_file_path` cannot represent a decoded null byte. - if urlencoding::decode_binary(url.path().as_bytes()).contains(&0) { + if urlencoding::decode_binary(url.path().as_bytes()).contains(&0) + && decode_bad_path_uri(url).is_none() + { return Err(PathUriParseError::InvalidFileUriPath); } Ok(()) diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index 4fb2ba5b452c..f4ad44ae9a55 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -1,9 +1,12 @@ use super::*; use codex_utils_absolute_path::AbsolutePathBufGuard; use pretty_assertions::assert_eq; +#[cfg(windows)] +use std::ffi::OsString; #[cfg(unix)] use std::os::unix::ffi::OsStringExt; -#[cfg(unix)] +#[cfg(windows)] +use std::os::windows::ffi::OsStringExt; use std::path::PathBuf; #[test] @@ -12,7 +15,7 @@ fn file_uri_round_trips_an_absolute_path() { .expect("current directory") .join("a path/file.rs"); - let uri = PathUri::from_abs_path(&path).expect("path should convert to a file URI"); + let uri = PathUri::from_abs_path(&path); let uri_string = uri.to_string(); assert!(uri_string.starts_with("file:")); @@ -57,24 +60,136 @@ fn file_uri_parses_a_windows_path_on_any_host() { #[cfg(windows)] #[test] -fn file_uri_rejects_windows_prefixes_without_a_uri_representation() { - for native_path in [ - r"\\.\COM1", - r"\\?\Volume{00000000-0000-0000-0000-000000000000}\file.rs", +fn file_uri_falls_back_for_windows_prefixes_without_a_uri_representation() { + for (native_path, expected_uri) in [ + (r"\\.\COM1", "file:///%00/bad/path/XABcAC4AXABDAE8ATQAxAFwA"), + ( + r"\\?\Volume{00000000-0000-0000-0000-000000000000}\file.rs", + "file:///%00/bad/path/XABcAD8AXABWAG8AbAB1AG0AZQB7ADAAMAAwADAAMAAwADAAMAAtADAAMAAwADAALQAwADAAMAAwAC0AMAAwADAAMAAtADAAMAAwADAAMAAwADAAMAAwADAAMAAwAH0AXABmAGkAbABlAC4AcgBzAA", + ), ] { let path = AbsolutePathBuf::from_absolute_path_checked(native_path) .expect("Windows namespace path should be absolute"); + let uri = PathUri::from_abs_path(&path); + + assert_eq!(uri.to_string(), expected_uri, "converting {native_path}"); assert_eq!( - PathUri::from_abs_path(&path) - .expect_err("Windows namespace path should not convert") - .kind(), - io::ErrorKind::InvalidInput, - "converting {native_path}" + PathUri::parse(&uri.to_string()) + .expect("fallback URI should parse") + .to_abs_path() + .expect("fallback URI should decode"), + path, + "round-tripping {native_path}" ); } } +#[cfg(windows)] +#[test] +fn file_uri_fallback_round_trips_non_unicode_windows_paths() { + let path_wide = r"C:\bad\" + .encode_utf16() + .chain([0xd800]) + .collect::>(); + let path = PathBuf::from(OsString::from_wide(&path_wide)); + let path = AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute Windows path"); + + let uri = PathUri::from_abs_path(&path); + let reparsed = PathUri::parse(&uri.to_string()).expect("fallback URI should parse"); + + assert!(uri.to_string().starts_with(BAD_PATH_URI_PREFIX)); + assert_eq!( + reparsed.to_abs_path().expect("fallback URI should decode"), + path + ); +} + +#[cfg(unix)] +#[test] +fn file_uri_falls_back_for_posix_paths_with_null_bytes() { + let path = PathBuf::from(std::ffi::OsString::from_vec( + b"/tmp/null-\0-\xff-byte".to_vec(), + )); + let path = AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute POSIX path"); + + let uri = PathUri::from_abs_path(&path); + + assert_eq!( + uri, + PathUri::parse("file:///%00/bad/path/L3RtcC9udWxsLQAt_y1ieXRl") + .expect("valid fallback URI") + ); + let json = serde_json::to_string(&uri).expect("fallback URI should serialize"); + let reparsed: PathUri = + serde_json::from_str(&json).expect("serialized fallback URI should parse"); + assert_eq!(json, r#""file:///%00/bad/path/L3RtcC9udWxsLQAt_y1ieXRl""#); + assert_eq!(reparsed, uri); + assert_eq!( + reparsed.to_abs_path().expect("fallback URI should decode"), + path + ); +} + +#[cfg(unix)] +#[test] +fn ordinary_bad_path_uri_is_not_decoded_as_a_fallback() { + let path = AbsolutePathBuf::from_absolute_path_checked("/bad/path/L3RtcC9udWxsLQAt_y1ieXRl") + .expect("absolute POSIX path"); + let uri = PathUri::from_abs_path(&path); + + assert_eq!(uri.to_string(), "file:///bad/path/L3RtcC9udWxsLQAt_y1ieXRl"); + assert_eq!( + uri.to_abs_path().expect("URI should convert literally"), + path + ); +} + +#[test] +fn malformed_bad_path_uris_are_rejected() { + for uri in [ + "file:///%00/bad/path/", + "file:///%00/bad/path/not*base64", + "file:///%00/bad/path/YQ==", + "file:///%00/bad/path/YR", + "file:///%00/bad/path/YQ/extra", + "file:///%00/other/YQ", + ] { + assert_eq!( + PathUri::parse(uri), + Err(PathUriParseError::InvalidFileUriPath), + "parsing {uri}" + ); + } +} + +#[test] +fn structurally_valid_bad_path_uri_with_invalid_native_payload_fails_conversion() { + let uri = PathUri::parse("file:///%00/bad/path/YQ") + .expect("canonical base64 fallback URI should parse"); + + assert_eq!( + uri.to_abs_path() + .expect_err("relative fallback payload should not convert") + .kind(), + io::ErrorKind::InvalidInput + ); +} + +#[test] +fn bad_path_uris_are_opaque_to_lexical_operations() { + let uri = PathUri::parse("file:///%00/bad/path/YQ") + .expect("canonical base64 fallback URI should parse"); + + assert_eq!(uri.basename(), None); + assert_eq!(uri.parent(), None); + assert_eq!(uri.join(""), Ok(uri.clone())); + assert_eq!( + uri.join("child"), + Err(PathUriParseError::InvalidFileUriPath) + ); +} + #[test] fn file_uri_parses_a_posix_path_on_any_host() { let uri = PathUri::parse("file:///home/alice/src/main.rs") @@ -101,7 +216,7 @@ fn file_uri_accepts_non_utf8_posix_paths() { let path = PathBuf::from(std::ffi::OsString::from_vec(b"/tmp/non-utf8-\xff".to_vec())); let path = AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute POSIX path"); - let uri = PathUri::from_abs_path(&path).expect("non-UTF-8 path should convert to a file URI"); + let uri = PathUri::from_abs_path(&path); assert_eq!( uri.to_abs_path() .expect("URI should convert to native path"), @@ -127,7 +242,7 @@ fn file_uri_round_trips_literal_percent_characters() { fn file_uri_round_trips_windows_unc_paths() { let path = AbsolutePathBuf::from_absolute_path_checked(r"\\server\share\src\main.rs") .expect("absolute UNC path"); - let uri = PathUri::from_abs_path(&path).expect("UNC path should convert to a file URI"); + let uri = PathUri::from_abs_path(&path); assert_eq!(uri.encoded_path(), "/share/src/main.rs"); assert_eq!(uri.to_abs_path().expect("UNC URI should convert"), path); @@ -198,10 +313,7 @@ fn path_uri_deserializes_legacy_absolute_paths() { let json = serde_json::to_string(&path).expect("absolute path should serialize"); let uri: PathUri = serde_json::from_str(&json).expect("legacy absolute path should parse"); - assert_eq!( - uri, - PathUri::from_abs_path(&path).expect("expected file URI") - ); + assert_eq!(uri, PathUri::from_abs_path(&path)); } #[test] diff --git a/codex-rs/utils/plugins/src/plugin_namespace.rs b/codex-rs/utils/plugins/src/plugin_namespace.rs index 32ed608b358c..b29ff5c5d684 100644 --- a/codex-rs/utils/plugins/src/plugin_namespace.rs +++ b/codex-rs/utils/plugins/src/plugin_namespace.rs @@ -31,7 +31,7 @@ async fn plugin_manifest_name( let mut manifest_path = None; for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS { let candidate = plugin_root.join(relative_path); - let candidate_uri = PathUri::from_abs_path(&candidate).ok()?; + let candidate_uri = PathUri::from_abs_path(&candidate); match fs.get_metadata(&candidate_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_file => { manifest_path = Some(candidate); @@ -41,7 +41,7 @@ async fn plugin_manifest_name( } } let manifest_path = manifest_path?; - let manifest_path_uri = PathUri::from_abs_path(&manifest_path).ok()?; + let manifest_path_uri = PathUri::from_abs_path(&manifest_path); let contents = fs .read_file_text(&manifest_path_uri, /*sandbox*/ None) .await