diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index 641f7e700a..62d901e0f4 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -283,7 +283,7 @@ impl EnvironmentInfo { environment_config_read: true, http_header_env_vars: true, sandboxed_file_streaming: true, - file_write_streaming: false, + file_write_streaming: true, shell_snapshot_v2: cfg!(unix), windows_mxc, linux_root_write_preserves_devices: cfg!(target_os = "linux"), diff --git a/codex-rs/exec-server/README.md b/codex-rs/exec-server/README.md index f32972604e..fc4419a9a2 100644 --- a/codex-rs/exec-server/README.md +++ b/codex-rs/exec-server/README.md @@ -434,8 +434,8 @@ for invalid or unavailable paths. Native absolute path strings are rejected; callers must convert them to `file:` URIs before sending requests: - `fs/readFile` -- `fs/open`, `fs/readBlock`, and `fs/close` (internal transport for - `ExecutorFileSystem::read_file_stream`) +- `fs/open`, `fs/readBlock`, `fs/writeBlock`, and `fs/close` (file handles; + reads also back `ExecutorFileSystem::read_file_stream`) - `fs/writeFile` - `fs/createDirectory` - `fs/getMetadata` @@ -444,7 +444,11 @@ callers must convert them to `file:` URIs before sending requests: - `fs/remove` - `fs/copy` -Each filesystem request accepts an optional `sandbox` object. When `sandbox` +`fs/open.mode` is `read` (default) or `replace` (create or truncate for writing). +Writable handles require `fileWriteStreaming`; `fs/writeBlock` writes a nonempty +base64 `chunk` of up to 1 MiB at an explicit `offset`. + +Path-based filesystem requests accept an optional `sandbox` object. When `sandbox` contains a `ReadOnly` or `WorkspaceWrite` policy, the operation runs in a hidden helper process launched from the top-level `codex` executable and prepared through the shared sandbox transform path. Helper requests and diff --git a/codex-rs/exec-server/src/discoverV2/capability_locations.rs b/codex-rs/exec-server/src/discoverV2/capability_locations.rs index 04f6a373e0..5f5a397400 100644 --- a/codex-rs/exec-server/src/discoverV2/capability_locations.rs +++ b/codex-rs/exec-server/src/discoverV2/capability_locations.rs @@ -7,6 +7,7 @@ use std::collections::BTreeMap; use std::io; use crate::LocalFileSystem; +use crate::protocol::FsOpenMode; use codex_config::loader::system_config_toml_file; use codex_file_system::ExecutorFileSystem; use codex_file_system::WalkEntryKind; @@ -182,7 +183,7 @@ async fn discover_cached_plugins( async fn read_remote_plugin_id(path: &std::path::Path) -> io::Result> { // Nonblocking open and handle validation reject FIFOs and other non-regular files. - let file = match crate::regular_file::open(path).await { + let file = match crate::regular_file::open(path, FsOpenMode::Read).await { Ok(file) => file, Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), Err(error) => return Err(error), diff --git a/codex-rs/exec-server/src/file_handle.rs b/codex-rs/exec-server/src/file_handle.rs index dc1011b28f..a180b5696d 100644 --- a/codex-rs/exec-server/src/file_handle.rs +++ b/codex-rs/exec-server/src/file_handle.rs @@ -1,4 +1,4 @@ -//! Connection-scoped file handles with bounded positional reads. +//! Connection-scoped file handles with bounded positional reads and writes. //! Semaphore permits bound open handles and in-flight opens; table locks never cross an await. use std::collections::HashMap; @@ -11,6 +11,7 @@ use std::sync::Mutex; use std::sync::MutexGuard; use codex_file_system::FILE_READ_CHUNK_SIZE; +use codex_file_system::FILE_WRITE_CHUNK_SIZE; use tokio::sync::OwnedSemaphorePermit; use tokio::sync::Semaphore; @@ -46,13 +47,13 @@ impl FileHandleManager { if self.lock_handles().contains_key(&handle_id) { return Err(io::Error::new( io::ErrorKind::InvalidInput, - format!("file read handle `{handle_id}` already exists"), + format!("file handle `{handle_id}` already exists"), )); } let permit = Arc::clone(&self.slots).try_acquire_owned().map_err(|_| { io::Error::new( io::ErrorKind::InvalidInput, - format!("at most {MAX_OPEN_FILES} file reads may be open per connection"), + format!("at most {MAX_OPEN_FILES} file handles may be open per connection"), ) })?; let file = Arc::new(open_file.await?.into_std().await); @@ -60,7 +61,7 @@ impl FileHandleManager { let Entry::Vacant(entry) = handles.entry(handle_id.clone()) else { return Err(io::Error::new( io::ErrorKind::InvalidInput, - format!("file read handle `{handle_id}` already exists"), + format!("file handle `{handle_id}` already exists"), )); }; entry.insert(FileHandleEntry { @@ -91,6 +92,65 @@ impl FileHandleManager { result } + pub(crate) async fn write_block( + &self, + handle_id: &str, + offset: u64, + bytes: Vec, + ) -> io::Result<()> { + if !(1..=FILE_WRITE_CHUNK_SIZE).contains(&bytes.len()) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("file write block length must be between 1 and {FILE_WRITE_CHUNK_SIZE}"), + )); + } + // Native file offsets are signed; Windows interprets negative offsets as sentinels. + let end = offset + .checked_add(bytes.len() as u64) + .and_then(|end| i64::try_from(end).ok()) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + "file write range exceeds the signed 64-bit file offset limit", + ) + })? as u64; + let file = self.get(handle_id)?; + tokio::task::spawn_blocking(move || { + let mut position = offset; + while position < end { + let written = (position - offset) as usize; + #[cfg(unix)] + let result = std::os::unix::fs::FileExt::write_at( + file.as_ref(), + &bytes[written..], + position, + ); + #[cfg(windows)] + let result = std::os::windows::fs::FileExt::seek_write( + file.as_ref(), + &bytes[written..], + position, + ); + match result { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "failed to write file block", + )); + } + Ok(count) => position += count as u64, + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + Err(error) => return Err(error), + } + } + Ok(()) + }) + .await + .map_err(|error| { + io::Error::other(format!("file write task stopped unexpectedly: {error}")) + })? + } + fn get(&self, handle_id: &str) -> io::Result> { self.lock_handles() .get(handle_id) @@ -115,7 +175,7 @@ impl FileHandleManager { struct FileHandleEntry { file: Arc, - // Closing an entry releases capacity even if a read still holds the file. + // Closing an entry releases capacity even if a read or write still holds the file. _permit: OwnedSemaphorePermit, } @@ -163,7 +223,7 @@ fn validate_read_block_len(len: usize) -> io::Result<()> { fn unknown_handle_error(handle_id: &str) -> io::Error { io::Error::new( io::ErrorKind::NotFound, - format!("unknown file read handle `{handle_id}`"), + format!("unknown file handle `{handle_id}`"), ) } diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index 2514ff5dfe..d30766370c 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -39,6 +39,7 @@ use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsCreateDirectoryResponse; use crate::protocol::FsGetMetadataParams; use crate::protocol::FsGetMetadataResponse; +use crate::protocol::FsOpenMode; use crate::protocol::FsReadDirectoryEntry; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadDirectoryResponse; @@ -92,6 +93,8 @@ pub(crate) enum FsHelperRequest { #[serde(rename_all = "camelCase")] pub(crate) struct FsHelperOpenParams { pub(crate) path: PathUri, + #[serde(default)] + pub(crate) mode: FsOpenMode, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/codex-rs/exec-server/src/fs_helper_main.rs b/codex-rs/exec-server/src/fs_helper_main.rs index 3ca9ecc21e..c5138385fc 100644 --- a/codex-rs/exec-server/src/fs_helper_main.rs +++ b/codex-rs/exec-server/src/fs_helper_main.rs @@ -43,7 +43,7 @@ async fn run_main() -> Result<(), Box> { FsHelperRequest::Open(params) => { let result: io::Result<_> = async { let path = params.path.to_abs_path()?; - let file = regular_file::open(path.as_path()).await?; + let file = regular_file::open(path.as_path(), params.mode).await?; // Unix can hand the opened fd directly to the parent. #[cfg(unix)] crate::sandboxed_file_open::transfer_file(&file)?; diff --git a/codex-rs/exec-server/src/fs_sandbox_windows_tests.rs b/codex-rs/exec-server/src/fs_sandbox_windows_tests.rs index fce05b15b9..099d594623 100644 --- a/codex-rs/exec-server/src/fs_sandbox_windows_tests.rs +++ b/codex-rs/exec-server/src/fs_sandbox_windows_tests.rs @@ -208,6 +208,7 @@ $handle = $file.SafeFileHandle.DangerousGetHandle().ToInt64() crate::sandboxed_file_open::open( command, PathUri::from_host_native_path(&path).expect("image path URI"), + crate::protocol::FsOpenMode::Read, ), ) .await diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 7e3c7ef899..ec8b81c52a 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -38,6 +38,7 @@ use crate::WalkOptions; use crate::WalkOutcome; use crate::WriteFileOptions; use crate::no_follow; +use crate::protocol::FsOpenMode; use crate::regular_file; use crate::sandboxed_file_system::SandboxedFileSystem; @@ -121,15 +122,20 @@ impl LocalFileSystem { pub(crate) async fn open_file( &self, path: &PathUri, + mode: FsOpenMode, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { if let Some(sandbox) = sandbox { sandbox.validate_file_system_paths_for_current_host()?; + let needs_sandbox = match mode { + FsOpenMode::Read => sandbox.should_read_from_sandbox(), + FsOpenMode::Replace => sandbox.should_write_into_sandbox(), + }; + if needs_sandbox { + return self.sandboxed()?.open_file(path, mode, Some(sandbox)).await; + } } - if sandbox.is_some_and(FileSystemSandboxContext::should_read_from_sandbox) { - return self.sandboxed()?.open_file(path, sandbox).await; - } - self.unsandboxed.open_file(path, /*sandbox*/ None).await + regular_file::open(path.to_abs_path()?.as_path(), mode).await } async fn canonicalize( @@ -348,15 +354,6 @@ impl ExecutorFileSystem for LocalFileSystem { } impl UnsandboxedFileSystem { - async fn open_file( - &self, - path: &PathUri, - sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { - reject_platform_sandbox_context(sandbox)?; - self.file_system.open_file(path, /*sandbox*/ None).await - } - async fn canonicalize( &self, path: &PathUri, @@ -584,7 +581,7 @@ impl DirectFileSystem { ) -> FileSystemResult { reject_sandbox_context(sandbox)?; let path = path.to_abs_path()?; - regular_file::open(path.as_path()).await + regular_file::open(path.as_path(), FsOpenMode::Read).await } async fn canonicalize( diff --git a/codex-rs/exec-server/src/local_file_system_read.rs b/codex-rs/exec-server/src/local_file_system_read.rs index 9908ff18d4..e9bd05e85c 100644 --- a/codex-rs/exec-server/src/local_file_system_read.rs +++ b/codex-rs/exec-server/src/local_file_system_read.rs @@ -3,6 +3,7 @@ use crate::FILE_READ_CHUNK_SIZE; use crate::ReadFileOptions; use crate::no_follow; +use crate::protocol::FsOpenMode; use crate::regular_file; use codex_utils_absolute_path::AbsolutePathBuf; use std::io; @@ -22,7 +23,7 @@ pub(super) async fn read_file( return Err(io::Error::other("file read cancelled")); } let file = if options.follow_symlinks { - regular_file::open_sync(path.as_path())? + regular_file::open_sync(path.as_path(), FsOpenMode::Read)? } else { no_follow::open_file(path.as_path())? }; diff --git a/codex-rs/exec-server/src/regular_file.rs b/codex-rs/exec-server/src/regular_file.rs index 2e74373242..84d38bd381 100644 --- a/codex-rs/exec-server/src/regular_file.rs +++ b/codex-rs/exec-server/src/regular_file.rs @@ -6,17 +6,26 @@ use std::os::windows::fs::OpenOptionsExt; use std::path::Path; use tokio::io::AsyncReadExt; -pub(crate) async fn open(path: &Path) -> io::Result { +use crate::protocol::FsOpenMode; + +pub(crate) async fn open(path: &Path, mode: FsOpenMode) -> io::Result { let path = path.to_path_buf(); - tokio::task::spawn_blocking(move || open_sync(&path).map(tokio::fs::File::from_std)) + tokio::task::spawn_blocking(move || open_sync(&path, mode).map(tokio::fs::File::from_std)) .await .map_err(|error| io::Error::other(format!("filesystem task failed: {error}")))? } /// Opens a regular file without blocking on Unix FIFOs or impersonating Windows pipe servers. -pub(crate) fn open_sync(path: &Path) -> io::Result { +pub(crate) fn open_sync(path: &Path, mode: FsOpenMode) -> io::Result { let mut options = std::fs::OpenOptions::new(); - options.read(true); + match mode { + FsOpenMode::Read => { + options.read(true); + } + FsOpenMode::Replace => { + options.write(true).create(true).truncate(true); + } + } configure_open(&mut options); let file = options.open(path)?; diff --git a/codex-rs/exec-server/src/sandboxed_file_open.rs b/codex-rs/exec-server/src/sandboxed_file_open.rs index 18e78e1746..bf21a065cc 100644 --- a/codex-rs/exec-server/src/sandboxed_file_open.rs +++ b/codex-rs/exec-server/src/sandboxed_file_open.rs @@ -18,14 +18,16 @@ use crate::fs_sandbox::reap_helper_after_response; use crate::fs_sandbox::spawn_command; #[cfg(unix)] use crate::fs_sandbox::wait_for_helper_output; +use crate::protocol::FsOpenMode; use crate::rpc::internal_error; use crate::rpc::invalid_request; pub(crate) async fn open( command: SandboxExecRequest, path: PathUri, + mode: FsOpenMode, ) -> Result { - let request = serde_json::to_vec(&FsHelperRequest::Open(FsHelperOpenParams { path })) + let request = serde_json::to_vec(&FsHelperRequest::Open(FsHelperOpenParams { path, mode })) .map_err(|error| internal_error(format!("invalid fs sandbox helper request: {error}")))?; open_platform(command, request).await } diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 24b8a240f1..374058f0d0 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -33,6 +33,7 @@ use crate::protocol::FsCanonicalizeParams; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; +use crate::protocol::FsOpenMode; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadFileParams; use crate::protocol::FsRemoveParams; @@ -83,6 +84,7 @@ impl SandboxedFileSystem { pub(crate) async fn open_file( &self, path: &PathUri, + mode: FsOpenMode, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { let sandbox = require_platform_sandbox(sandbox)?; @@ -91,7 +93,7 @@ impl SandboxedFileSystem { .sandbox_runner .sandbox_command(sandbox) .map_err(map_sandbox_error)?; - crate::sandboxed_file_open::open(command, path.clone()) + crate::sandboxed_file_open::open(command, path.clone(), mode) .await .map_err(map_sandbox_error) } @@ -367,7 +369,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { sandbox: Option<&'a FileSystemSandboxContext>, ) -> ExecutorFileSystemFuture<'a, FileSystemReadStream> { Box::pin(async move { - let file = self.open_file(path, sandbox).await?; + let file = self.open_file(path, FsOpenMode::Read, sandbox).await?; Ok(FileSystemReadStream::new(ReaderStream::with_capacity( file, FILE_READ_CHUNK_SIZE, 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 0d416a0bcd..199c1188fd 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -28,7 +28,6 @@ use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsCreateDirectoryResponse; use crate::protocol::FsGetMetadataParams; use crate::protocol::FsGetMetadataResponse; -use crate::protocol::FsOpenMode; use crate::protocol::FsOpenParams; use crate::protocol::FsOpenResponse; use crate::protocol::FsReadBlockParams; @@ -125,18 +124,12 @@ impl FileSystemHandler { params: FsOpenParams, ) -> Result { validate_file_handle_id(¶ms.handle_id)?; - // TODO(anp): Enable replacement opens when writable file streams are implemented. - if params.mode == FsOpenMode::Replace { - return Err(invalid_request( - "exec-server does not support writable file streams".to_string(), - )); - } let handle_id = self .file_handles .open( params.handle_id, self.file_system - .open_file(¶ms.path, params.sandbox.as_ref()), + .open_file(¶ms.path, params.mode, params.sandbox.as_ref()), ) .await .map_err(map_fs_error)?; @@ -164,10 +157,11 @@ impl FileSystemHandler { params: FsWriteBlockParams, ) -> Result { validate_file_handle_id(¶ms.handle_id)?; - // TODO(anp): Implement positional writes before advertising writable file streams. - Err(invalid_request( - "exec-server does not support writable file streams".to_string(), - )) + self.file_handles + .write_block(¶ms.handle_id, params.offset, params.chunk.into_inner()) + .await + .map_err(map_fs_error)?; + Ok(FsWriteBlockResponse {}) } pub(crate) async fn close( @@ -357,7 +351,7 @@ impl FileSystemHandler { fn validate_file_handle_id(handle_id: &str) -> Result<(), JSONRPCErrorError> { if handle_id.len() > MAX_FILE_HANDLE_ID_BYTES { return Err(invalid_request(format!( - "file read handle ID must not exceed {MAX_FILE_HANDLE_ID_BYTES} bytes" + "file handle ID must not exceed {MAX_FILE_HANDLE_ID_BYTES} bytes" ))); } Ok(()) @@ -383,7 +377,6 @@ mod tests { use super::*; use crate::FileSystemSandboxContext; use crate::protocol::FsReadFileParams; - use crate::protocol::FsWriteFileParams; #[tokio::test] diff --git a/codex-rs/exec-server/tests/file_stream.rs b/codex-rs/exec-server/tests/file_stream.rs index 3d7cb11d2d..2e37bb729b 100644 --- a/codex-rs/exec-server/tests/file_stream.rs +++ b/codex-rs/exec-server/tests/file_stream.rs @@ -12,6 +12,7 @@ use codex_exec_server::FsOpenMode; use codex_exec_server::FsOpenParams; use codex_exec_server::FsReadBlockParams; use codex_exec_server::FsReadBlockResponse; +use codex_exec_server::FsWriteBlockParams; use codex_exec_server::ReadFileOptions; use codex_exec_server::RemoteExecServerConnectArgs; use codex_http_client::HttpClientFactory; @@ -233,6 +234,15 @@ async fn read_block_supports_non_sequential_offsets_and_lengths() -> Result<()> }) .await?; + client + .fs_write_block(FsWriteBlockParams { + handle_id: open.handle_id.clone(), + offset: 0, + chunk: b"x".to_vec().into(), + }) + .await + .expect_err("default read-only handles must reject writes"); + let mut blocks = Vec::new(); for (offset, len) in [(6, 3), (1, 2), (8, 4), (0, 2)] { blocks.push( @@ -318,7 +328,7 @@ async fn open_enforces_the_per_connection_limit_and_close_releases_capacity() -> (code, message), ( -32600, - format!("at most {OPEN_FILE_LIMIT} file reads may be open per connection"), + format!("at most {OPEN_FILE_LIMIT} file handles may be open per connection"), ) ); @@ -340,6 +350,56 @@ async fn open_enforces_the_per_connection_limit_and_close_releases_capacity() -> Ok(()) } +/// Rejected replacement opens must neither truncate existing files nor create missing files. +#[test_case::test_case(1, "0", "file handle `0` already exists"; "duplicate_id")] +#[test_case::test_case(OPEN_FILE_LIMIT, "overflow", "at most 128 file handles may be open per connection"; "capacity")] +#[tokio::test] +async fn replace_open_rejects_unavailable_handles_before_touching_files( + handle_count: usize, + handle_id: &str, + expected_message: &str, +) -> Result<()> { + let server = exec_server().await?; + let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs::new( + server.websocket_url().to_string(), + "file-admission-test".to_string(), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + )) + .await?; + let tmp = TempDir::new()?; + let existing = tmp.path().join("existing.bin"); + let missing = tmp.path().join("missing.bin"); + std::fs::write(&existing, b"original")?; + for index in 0..handle_count { + client + .fs_open(FsOpenParams { + handle_id: index.to_string(), + path: PathUri::from_host_native_path(&existing)?, + mode: FsOpenMode::Read, + sandbox: None, + }) + .await?; + } + + for path in [&existing, &missing] { + let error = client + .fs_open(FsOpenParams { + handle_id: handle_id.to_string(), + path: PathUri::from_host_native_path(path)?, + mode: FsOpenMode::Replace, + sandbox: None, + }) + .await; + let Err(ExecServerError::Server { code, message }) = error else { + anyhow::bail!("expected server error, got {error:?}"); + }; + assert_eq!((code, message), (-32600, expected_message.to_string())); + } + assert_eq!(std::fs::read(&existing)?, b"original"); + assert!(!missing.exists()); + Ok(()) +} + #[tokio::test] async fn open_rejects_handle_ids_longer_than_32_bytes() -> Result<()> { let server = exec_server().await?; @@ -370,12 +430,254 @@ async fn open_rejects_handle_ids_longer_than_32_bytes() -> Result<()> { (code, message), ( -32600, - "file read handle ID must not exceed 32 bytes".to_string(), + "file handle ID must not exceed 32 bytes".to_string(), ) ); Ok(()) } +/// Writes use explicit offsets and preserve untouched bytes across multiple blocks. +#[tokio::test] +async fn write_blocks_support_non_sequential_offsets_and_enforce_bounds() -> Result<()> { + let server = exec_server().await?; + let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs::new( + server.websocket_url().to_string(), + "file-write-test".to_string(), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + )) + .await?; + let tmp = TempDir::new()?; + let path = tmp.path().join("blocks.bin"); + let open = client + .fs_open(FsOpenParams { + handle_id: "writer".to_string(), + path: PathUri::from_host_native_path(&path)?, + mode: FsOpenMode::Replace, + sandbox: None, + }) + .await?; + for (offset, chunk) in [ + (BLOCK_SIZE as u64, vec![b'z'; 3]), + (0, vec![b'a'; BLOCK_SIZE]), + (1, b"bc".to_vec()), + ] { + client + .fs_write_block(FsWriteBlockParams { + handle_id: open.handle_id.clone(), + offset, + chunk: chunk.into(), + }) + .await?; + } + for (offset, chunk) in [ + (0, Vec::new()), + (0, vec![b'x'; BLOCK_SIZE + 1]), + (u64::MAX, vec![b'x']), + ] { + client + .fs_write_block(FsWriteBlockParams { + handle_id: open.handle_id.clone(), + offset, + chunk: chunk.into(), + }) + .await + .expect_err("empty, oversized, and overflowing writes must be rejected"); + } + let read = client + .fs_read_block(FsReadBlockParams { + handle_id: open.handle_id.clone(), + offset: 0, + len: 4, + }) + .await; + read.expect_err("write-only handles must reject reads"); + let mut expected = vec![b'a'; BLOCK_SIZE]; + expected[1..3].copy_from_slice(b"bc"); + expected.extend_from_slice(b"zzz"); + assert_eq!(std::fs::read(&path)?, expected); + client + .fs_write_block(FsWriteBlockParams { + handle_id: "writer".to_string(), + offset: 0, + chunk: b"x".to_vec().into(), + }) + .await + .expect_err("read failures must close the handle"); + Ok(()) +} + +/// Invalid signed offsets must not reach Windows' current-position sentinel or mutate the file. +#[test_case::test_case(u64::MAX - 1, 1; "windows_current_position_sentinel")] +#[test_case::test_case(u64::MAX, 1; "unsigned_overflow")] +#[test_case::test_case(i64::MAX as u64, 1; "signed_end_overflow")] +#[test_case::test_case(i64::MAX as u64 - 1, 2; "block_crosses_signed_limit")] +#[tokio::test] +async fn write_blocks_reject_ranges_outside_signed_file_offsets( + offset: u64, + len: usize, +) -> Result<()> { + let server = exec_server().await?; + let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs::new( + server.websocket_url().to_string(), + "file-write-offset-test".to_string(), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + )) + .await?; + let tmp = TempDir::new()?; + let path = tmp.path().join("offsets.bin"); + let open = client + .fs_open(FsOpenParams { + handle_id: "writer".to_string(), + path: PathUri::from_host_native_path(&path)?, + mode: FsOpenMode::Replace, + sandbox: None, + }) + .await?; + client + .fs_write_block(FsWriteBlockParams { + handle_id: open.handle_id.clone(), + offset: 0, + chunk: b"original".to_vec().into(), + }) + .await?; + + let error = client + .fs_write_block(FsWriteBlockParams { + handle_id: open.handle_id.clone(), + offset, + chunk: vec![b'x'; len].into(), + }) + .await; + let Err(ExecServerError::Server { code, message }) = error else { + anyhow::bail!("expected server error, got {error:?}"); + }; + assert_eq!( + (code, message), + ( + -32600, + "file write range exceeds the signed 64-bit file offset limit".to_string(), + ) + ); + client + .fs_close(FsCloseParams { + handle_id: open.handle_id, + }) + .await?; + assert_eq!(std::fs::read(&path)?, b"original"); + Ok(()) +} + +/// Replacement discards the previous contents before any streamed blocks are written. +#[tokio::test] +async fn replace_open_truncates_existing_file() -> Result<()> { + let server = exec_server().await?; + let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs::new( + server.websocket_url().to_string(), + "file-replace-test".to_string(), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + )) + .await?; + let tmp = TempDir::new()?; + let path = tmp.path().join("replace.bin"); + std::fs::write(&path, b"original")?; + let open = client + .fs_open(FsOpenParams { + handle_id: "replacement".to_string(), + path: PathUri::from_host_native_path(&path)?, + mode: FsOpenMode::Replace, + sandbox: None, + }) + .await?; + assert_eq!(std::fs::read(&path)?, b""); + client + .fs_close(FsCloseParams { + handle_id: open.handle_id, + }) + .await?; + Ok(()) +} + +/// Both restricted reads and full-disk reads must retain write sandbox enforcement. +#[cfg(unix)] +#[tokio::test] +async fn writable_open_obeys_sandbox_write_permissions() -> Result<()> { + use codex_exec_server::FileSystemSandboxContext; + use codex_protocol::models::PermissionProfile; + use codex_protocol::permissions::FileSystemAccessMode; + use codex_protocol::permissions::FileSystemPath; + use codex_protocol::permissions::FileSystemSandboxEntry; + use codex_protocol::permissions::FileSystemSandboxPolicy; + use codex_protocol::permissions::FileSystemSpecialPath; + use codex_protocol::permissions::NetworkSandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; + + let server = exec_server().await?; + let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs::new( + server.websocket_url().to_string(), + "file-write-sandbox-test".to_string(), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + )) + .await?; + let workspace = TempDir::new()?; + let outside = TempDir::new()?; + let allowed = workspace.path().join("allowed.bin"); + let denied = outside.path().join("denied.bin"); + std::fs::write(&denied, b"original")?; + for full_disk_read in [false, true] { + let mut entries = vec![FileSystemSandboxEntry::new( + AbsolutePathBuf::from_absolute_path(workspace.path())?.into(), + FileSystemAccessMode::Write, + )]; + if full_disk_read { + entries.push(FileSystemSandboxEntry::new( + FileSystemPath::Special { + value: FileSystemSpecialPath::Root, + }, + FileSystemAccessMode::Read, + )); + } + let sandbox = FileSystemSandboxContext::from_permission_profile( + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(entries), + NetworkSandboxPolicy::Restricted, + ), + PathUri::from_host_native_path(workspace.path())?, + ); + client + .fs_open(FsOpenParams { + handle_id: "denied".to_string(), + path: PathUri::from_host_native_path(&denied)?, + mode: FsOpenMode::Replace, + sandbox: Some(sandbox.clone()), + }) + .await + .expect_err("writable opens must not escape the write sandbox"); + let open = client + .fs_open(FsOpenParams { + handle_id: "allowed".to_string(), + path: PathUri::from_host_native_path(&allowed)?, + mode: FsOpenMode::Replace, + sandbox: Some(sandbox.clone()), + }) + .await?; + client + .fs_write_block(FsWriteBlockParams { + handle_id: open.handle_id.clone(), + offset: 0, + chunk: b"allowed".to_vec().into(), + }) + .await?; + client + .fs_close(FsCloseParams { + handle_id: open.handle_id, + }) + .await?; + assert_eq!(std::fs::read(&allowed)?, b"allowed"); + assert_eq!(std::fs::read(&denied)?, b"original"); + } + Ok(()) +} + fn connect_file_system(websocket_url: &str) -> Result> { let environment = Environment::create_for_tests(Some(websocket_url.to_string()))?; Ok(environment.get_filesystem())