From 638370be904ecefcf5e4660814cadcf6cc79e507 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Fri, 10 Jul 2026 06:01:31 +0000 Subject: [PATCH] fix(exec-server): harden text-prefix transport --- codex-rs/exec-server-protocol/src/protocol.rs | 5 -- codex-rs/exec-server/src/lib.rs | 1 - .../src/remote_text_prefix_tests.rs | 9 ++++ .../src/server/file_system_handler.rs | 2 + .../exec-server/tests/file_system_unix.rs | 47 +++++++++++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index 1e2770c6de..d0793f8c08 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -275,11 +275,6 @@ pub struct FsReadFileResponse { pub const FS_READ_TEXT_PREFIXES_BATCH_MAX_PATHS: usize = 64; /// Maximum caller-selected raw prefix size for one `fs/readTextPrefixesBatch` item. pub const FS_READ_TEXT_PREFIXES_BATCH_MAX_PREFIX_BYTES: usize = 16 * 1024; -/// Maximum raw prefix bytes returned by one full batch. Per-item base64 encoding expands this to -/// at most 1,398,272 bytes before per-item JSON framing. -pub const FS_READ_TEXT_PREFIXES_BATCH_MAX_RAW_RESPONSE_BYTES: usize = - FS_READ_TEXT_PREFIXES_BATCH_MAX_PATHS * FS_READ_TEXT_PREFIXES_BATCH_MAX_PREFIX_BYTES; - #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct FsReadTextPrefixesBatchParams { diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index ec21704f42..6c37cf1ba0 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -115,7 +115,6 @@ pub use protocol::FS_FIND_UP_BATCH_MAX_REQUESTS; pub use protocol::FS_GET_METADATA_BATCH_MAX_PATHS; pub use protocol::FS_READ_TEXT_PREFIXES_BATCH_MAX_PATHS; pub use protocol::FS_READ_TEXT_PREFIXES_BATCH_MAX_PREFIX_BYTES; -pub use protocol::FS_READ_TEXT_PREFIXES_BATCH_MAX_RAW_RESPONSE_BYTES; pub use protocol::FsCanonicalizeParams; pub use protocol::FsCanonicalizeResponse; pub use protocol::FsCloseParams; diff --git a/codex-rs/exec-server/src/remote_text_prefix_tests.rs b/codex-rs/exec-server/src/remote_text_prefix_tests.rs index 6e5715cf2b..b38c1c55a7 100644 --- a/codex-rs/exec-server/src/remote_text_prefix_tests.rs +++ b/codex-rs/exec-server/src/remote_text_prefix_tests.rs @@ -16,6 +16,15 @@ fn rejects_malformed_cardinality_base64_and_utf8() { } } +#[test] +fn rejects_prefix_larger_than_the_requested_decoded_limit() { + let error = decode_response(data_response("YWJjZGU="), 1, 4) + .expect_err("oversized decoded prefix should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidData); + assert!(error.to_string().contains("oversized prefix")); +} + fn data_response(data_base64: &str) -> FsReadTextPrefixesBatchResponse { FsReadTextPrefixesBatchResponse { results: vec![FsReadTextPrefixesBatchResult::Data { 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 e870da1081..4b217766bf 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -151,6 +151,8 @@ impl FileSystemHandler { } let mut results = Vec::with_capacity(params.paths.len()); for path in params.paths { + // Decode the complete file before selecting its prefix so invalid UTF-8 in an omitted + // tail has the same result as the existing complete-text read operation. let result = self .file_system .read_file(&path, params.sandbox.as_ref()) diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index bfcf9df601..e39b1cd712 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -25,6 +25,7 @@ use codex_exec_server::Environment; use codex_exec_server::FileMetadata; use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::RemoveOptions; +use codex_exec_server::TextFilePrefix; use codex_exec_server::WalkEntry; use codex_exec_server::WalkEntryKind; use codex_exec_server::WalkOptions; @@ -564,6 +565,52 @@ async fn file_system_sandboxed_read_rejects_symlink_escape( Ok(()) } +#[test_case(FileSystemImplementation::Local ; "local")] +#[test_case(FileSystemImplementation::Remote ; "remote")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn text_prefix_batch_preserves_order_and_restrictive_sandbox( + implementation: FileSystemImplementation, +) -> Result<()> { + let context = create_file_system_context(implementation).await?; + let file_system = context.file_system; + let tmp = TempDir::new()?; + let allowed_dir = tmp.path().join("allowed"); + let forbidden_dir = tmp.path().join("forbidden"); + let allowed_path = allowed_dir.join("allowed.txt"); + let forbidden_path = forbidden_dir.join("forbidden.txt"); + std::fs::create_dir_all(&allowed_dir)?; + std::fs::create_dir_all(&forbidden_dir)?; + std::fs::write(&allowed_path, "allowed")?; + std::fs::write(&forbidden_path, "forbidden")?; + let paths = vec![ + PathUri::from_host_native_path(&allowed_path)?, + PathUri::from_host_native_path(&forbidden_path)?, + ]; + let sandbox = read_only_sandbox(allowed_dir); + + let mut results = file_system + .read_text_prefixes_batch(&paths, 16, Some(&sandbox)) + .await + .with_context(|| format!("mode={implementation}"))? + .into_iter(); + + assert_eq!( + results.next().expect("allowed result")?, + TextFilePrefix { + text: "allowed".to_string(), + complete: true, + } + ); + let forbidden_error = results + .next() + .expect("forbidden result") + .expect_err("forbidden path must not return a prefix"); + assert_sandbox_denied(&forbidden_error); + assert!(results.next().is_none()); + + Ok(()) +} + #[test_case(FileSystemImplementation::Local ; "local")] #[test_case(FileSystemImplementation::Remote ; "remote")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]