diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index cd2cc38cb..5b42f9a2f 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -154,11 +154,8 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result Result { - cwd.to_abs_path().map_err(|err| { - invalid_request(format!( - "file system sandbox cwd is not native to this exec-server host: {err}" - )) - }) + cwd.to_abs_path() + .map_err(|err| invalid_request(err.to_string())) } fn helper_read_roots(runtime_paths: &ExecServerRuntimePaths) -> Vec { @@ -561,16 +558,16 @@ mod tests { FileSystemSpecialPath::project_roots(/*subpath*/ None), FileSystemAccessMode::Write, )]); - let sandbox_context = sandbox_context_with_cwd(&policy, cwd); + let sandbox_context = sandbox_context_with_cwd(&policy, cwd.clone()); let err = sandbox_cwd(&sandbox_context).expect_err("non-native cwd should be rejected"); assert_eq!( err, - crate::rpc::invalid_request( - "file system sandbox cwd is not native to this exec-server host: file URI contains an invalid absolute path" - .to_string() - ) + crate::rpc::invalid_request(format!( + "'{cwd}' is invalid on '{}'", + std::env::consts::OS + )) ); } diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index e9a00543f..c37ad31b4 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -211,13 +211,17 @@ impl PathUri { return Err(PathUriParseError::JoinPathMustBeRelative(path.to_string())); } if path.contains('\0') { - return Err(PathUriParseError::InvalidFileUriPath); + return Err(PathUriParseError::InvalidFileUriPath { + path: path.to_string(), + }); } if path.is_empty() { return Ok(self.clone()); } if decode_bad_path_uri(&self.0).is_some() { - return Err(PathUriParseError::InvalidFileUriPath); + return Err(PathUriParseError::InvalidFileUriPath { + path: self.to_string(), + }); } let mut url = self.0.clone(); @@ -280,18 +284,28 @@ impl PathUri { return Err(io::Error::new( io::ErrorKind::InvalidInput, - PathUriParseError::InvalidFileUriPath, + PathUriParseError::InvalidFileUriPath { + path: self.to_string(), + }, )); } let path = self.0.to_file_path().map_err(|()| { io::Error::new( io::ErrorKind::InvalidInput, - PathUriParseError::InvalidFileUriPath, + PathUriParseError::InvalidFileUriPath { + path: self.to_string(), + }, ) })?; - AbsolutePathBuf::from_absolute_path_checked(path) - .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err)) + AbsolutePathBuf::from_absolute_path_checked(path).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidInput, + PathUriParseError::InvalidFileUriPath { + path: self.to_string(), + }, + ) + }) } /// Returns a clone of the canonical URL. @@ -472,7 +486,9 @@ fn validate_file_url(url: &Url) -> Result<(), PathUriParseError> { if urlencoding::decode_binary(url.path().as_bytes()).contains(&0) && decode_bad_path_uri(url).is_none() { - return Err(PathUriParseError::InvalidFileUriPath); + return Err(PathUriParseError::InvalidFileUriPath { + path: url.to_string(), + }); } Ok(()) } @@ -483,8 +499,8 @@ pub enum PathUriParseError { InvalidUri(#[from] url::ParseError), #[error("unsupported path URI scheme `{0}`")] UnsupportedScheme(String), - #[error("file URI contains an invalid absolute path")] - InvalidFileUriPath, + #[error("'{path}' is invalid on '{os}'", os = std::env::consts::OS)] + InvalidFileUriPath { path: String }, #[error("credentials are not allowed in path URIs")] CredentialsNotAllowed, #[error("ports are not allowed in path URIs")] diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index fa0c5d8b0..fe436476d 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -42,7 +42,13 @@ fn non_native_uri_io_conversion_is_invalid_input() { .to_abs_path() .expect_err("URI should not be host-native"); - assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + assert_eq!( + (error.kind(), error.to_string()), + ( + io::ErrorKind::InvalidInput, + format!("'{uri}' is invalid on '{}'", std::env::consts::OS), + ) + ); } #[test] @@ -193,7 +199,9 @@ fn malformed_bad_path_uris_are_rejected() { ] { assert_eq!( PathUri::parse(uri), - Err(PathUriParseError::InvalidFileUriPath), + Err(PathUriParseError::InvalidFileUriPath { + path: uri.to_string(), + }), "parsing {uri}" ); } @@ -222,7 +230,9 @@ fn bad_path_uris_are_opaque_to_lexical_operations() { assert_eq!(uri.join(""), Ok(uri.clone())); assert_eq!( uri.join("child"), - Err(PathUriParseError::InvalidFileUriPath) + Err(PathUriParseError::InvalidFileUriPath { + path: uri.to_string(), + }) ); } @@ -497,10 +507,12 @@ fn join_rejects_absolute_and_null_paths() { base.join("/src"), Err(PathUriParseError::JoinPathMustBeRelative(path)) if path == "/src" )); - assert!(matches!( + assert_eq!( base.join("src\0file"), - Err(PathUriParseError::InvalidFileUriPath) - )); + Err(PathUriParseError::InvalidFileUriPath { + path: "src\0file".to_string(), + }) + ); } #[test]