diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 51a6cd08cc..0708006d1b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4131,7 +4131,9 @@ dependencies = [ "codex-utils-cache", "divan", "image", + "libc", "mime_guess", + "tempfile", "thiserror 2.0.18", "tokio", ] diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 535aa3c9e9..0f71bb99ba 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1,13 +1,13 @@ use std::collections::HashMap; use std::io; -use std::io::Read; use std::num::NonZeroUsize; use std::path::Path; +#[cfg(test)] use codex_utils_image::MAX_PROMPT_IMAGE_FILE_BYTES; use codex_utils_image::PromptImageMode; use codex_utils_image::load_for_prompt_bytes; -use codex_utils_image::validate_prompt_image_file_size; +use codex_utils_image::read_prompt_image_file; use serde::Deserialize; use serde::Deserializer; use serde::Serialize; @@ -1258,24 +1258,7 @@ impl From> for ResponseInputItem { UserInput::LocalImage { path, detail, .. } => { image_index += 1; let detail = detail.unwrap_or(DEFAULT_IMAGE_DETAIL); - let file_bytes = std::fs::metadata(&path).and_then(|metadata| { - if !metadata.is_file() { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - "local image path is not a regular file", - )); - } - validate_prompt_image_file_size(metadata.len())?; - - let capacity = usize::try_from(metadata.len()).unwrap_or_default(); - let mut file_bytes = Vec::with_capacity(capacity); - std::fs::File::open(&path)? - .take(MAX_PROMPT_IMAGE_FILE_BYTES + 1) - .read_to_end(&mut file_bytes)?; - let file_size = u64::try_from(file_bytes.len()).unwrap_or(u64::MAX); - validate_prompt_image_file_size(file_size)?; - Ok(file_bytes) - }); + let file_bytes = read_prompt_image_file(&path); match file_bytes { Ok(file_bytes) => local_image_content_items_with_label_number( &path, diff --git a/codex-rs/utils/image/Cargo.toml b/codex-rs/utils/image/Cargo.toml index 7ba28f4996..f1786b728b 100644 --- a/codex-rs/utils/image/Cargo.toml +++ b/codex-rs/utils/image/Cargo.toml @@ -15,9 +15,13 @@ mime_guess = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["fs", "rt", "rt-multi-thread", "macros"] } +[target.'cfg(unix)'.dependencies] +libc = { workspace = true } + [dev-dependencies] divan = { workspace = true } image = { workspace = true, features = ["jpeg", "png", "gif", "webp"] } +tempfile = { workspace = true } [lib] doctest = false diff --git a/codex-rs/utils/image/src/lib.rs b/codex-rs/utils/image/src/lib.rs index e28334889a..4dc7109df9 100644 --- a/codex-rs/utils/image/src/lib.rs +++ b/codex-rs/utils/image/src/lib.rs @@ -1,8 +1,13 @@ +use std::fs::OpenOptions; use std::io; +use std::io::Read; use std::num::NonZeroUsize; use std::path::Path; use std::sync::LazyLock; +#[cfg(unix)] +use std::os::unix::fs::OpenOptionsExt; + use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_utils_cache::BlockingLruCache; @@ -19,6 +24,8 @@ use image::imageops::FilterType; /// Maximum width or height used when resizing images before uploading. pub const MAX_DIMENSION: u32 = 2048; /// Maximum compressed file size accepted for an image added to a prompt. +/// +/// 50 MiB accommodates large source images while bounding the allocation made before decoding. pub const MAX_PROMPT_IMAGE_FILE_BYTES: u64 = 50 * 1024 * 1024; pub mod error; @@ -46,8 +53,7 @@ pub enum PromptImageMode { Original, } -/// Validates the compressed byte size of an image before prompt processing. -pub fn validate_prompt_image_file_size(file_size: u64) -> io::Result<()> { +fn validate_prompt_image_file_size(file_size: u64) -> io::Result<()> { if file_size > MAX_PROMPT_IMAGE_FILE_BYTES { return Err(io::Error::new( io::ErrorKind::InvalidData, @@ -57,6 +63,35 @@ pub fn validate_prompt_image_file_size(file_size: u64) -> io::Result<()> { Ok(()) } +/// Reads a regular image file into memory, up to the prompt image size limit. +/// +/// On Unix, the file is opened in nonblocking mode so a path resolving to a FIFO or other special +/// file cannot block before its type is checked. Metadata and bytes are read from the same handle. +pub fn read_prompt_image_file(path: &Path) -> io::Result> { + let mut options = OpenOptions::new(); + options.read(true); + #[cfg(unix)] + options.custom_flags(libc::O_NONBLOCK); + + let file = options.open(path)?; + let metadata = file.metadata()?; + if !metadata.is_file() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "local image path is not a regular file", + )); + } + validate_prompt_image_file_size(metadata.len())?; + + let capacity = usize::try_from(metadata.len()).unwrap_or_default(); + let mut file_bytes = Vec::with_capacity(capacity); + file.take(MAX_PROMPT_IMAGE_FILE_BYTES + 1) + .read_to_end(&mut file_bytes)?; + let file_size = u64::try_from(file_bytes.len()).unwrap_or(u64::MAX); + validate_prompt_image_file_size(file_size)?; + Ok(file_bytes) +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] struct ImageCacheKey { digest: [u8; 20], @@ -217,6 +252,15 @@ fn format_to_mime(format: ImageFormat) -> String { mod tests { use std::io::Cursor; + #[cfg(unix)] + use std::ffi::CString; + #[cfg(unix)] + use std::os::unix::ffi::OsStrExt; + #[cfg(unix)] + use std::sync::mpsc; + #[cfg(unix)] + use std::time::Duration; + use super::*; use image::GenericImageView; use image::ImageBuffer; @@ -245,6 +289,39 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn prompt_image_file_read_rejects_fifo_without_blocking() { + let dir = tempfile::tempdir().expect("create tempdir"); + let fifo_path = dir.path().join("image.png"); + let c_path = CString::new(fifo_path.as_os_str().as_bytes()).expect("path without nul"); + // SAFETY: `c_path` is NUL-terminated and remains valid for the duration of the call. + let result = unsafe { + libc::mkfifo(c_path.as_ptr(), /*mode*/ 0o600) + }; + assert_eq!(result, 0); + + let (sender, receiver) = mpsc::channel(); + let read_thread = std::thread::spawn(move || { + sender + .send(read_prompt_image_file(&fifo_path)) + .expect("send read result"); + }); + let read_result = receiver + .recv_timeout(Duration::from_secs(/*secs*/ 5)) + .expect("FIFO read should return without blocking"); + read_thread.join().expect("join read thread"); + + let error = read_result.expect_err("reject FIFO"); + assert_eq!( + (error.kind(), error.to_string()), + ( + io::ErrorKind::InvalidInput, + "local image path is not a regular file".to_string(), + ) + ); + } + #[tokio::test(flavor = "multi_thread")] async fn returns_original_image_when_within_bounds() { for (format, mime) in [