From 5055167b823ea22520bb58f02e7152122ec769e2 Mon Sep 17 00:00:00 2001 From: colby-oai <228809017+colby-oai@users.noreply.github.com> Date: Tue, 28 Apr 2026 16:26:10 -0400 Subject: [PATCH] clean up redundant old check --- codex-rs/Cargo.lock | 1 + codex-rs/exec-server/Cargo.toml | 3 ++ codex-rs/exec-server/src/fs_helper.rs | 34 +++++++++++++++++-- codex-rs/exec-server/src/fs_sandbox.rs | 4 +-- codex-rs/exec-server/src/local_file_system.rs | 4 +-- .../exec-server/src/sandboxed_file_system.rs | 32 ++--------------- codex-rs/exec-server/tests/file_system.rs | 12 ++++++- 7 files changed, 50 insertions(+), 40 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index eaee6ed294..879541120e 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2621,6 +2621,7 @@ dependencies = [ "codex-utils-pty", "ctor 0.6.3", "futures", + "libc", "pretty_assertions", "reqwest", "serde", diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 4dc77f8552..8615713f51 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -43,6 +43,9 @@ tokio-tungstenite = { workspace = true } tracing = { workspace = true } uuid = { workspace = true, features = ["v4"] } +[target.'cfg(unix)'.dependencies] +libc = { workspace = true } + [dev-dependencies] anyhow = { workspace = true } codex-test-binary-support = { workspace = true } diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index d663557f8f..faa9615cc6 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -3,6 +3,7 @@ use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::JSONRPCErrorError; use serde::Deserialize; use serde::Serialize; +use std::path::Path; use tokio::io; use tokio::io::AsyncWrite; @@ -202,7 +203,7 @@ pub(crate) async fn run_direct_request( })) } FsHelperRequest::ReadFileInfo(params) => { - let metadata = tokio::fs::metadata(params.path.as_path()) + let metadata = tokio::fs::symlink_metadata(params.path.as_path()) .await .map_err(map_fs_error)?; validate_read_file_metadata(params.path.as_path(), &metadata).map_err(map_fs_error)?; @@ -306,9 +307,9 @@ pub(crate) async fn run_direct_stream_request( ) -> Result<(), Box> { match request { FsHelperRequest::ReadFileStream(params) => { - let metadata = tokio::fs::metadata(params.path.as_path()).await?; + let metadata = tokio::fs::symlink_metadata(params.path.as_path()).await?; validate_read_file_metadata(params.path.as_path(), &metadata)?; - let mut file = tokio::fs::File::open(params.path.as_path()).await?; + let mut file = open_read_file_no_follow(params.path.as_path()).await?; tokio::io::copy(&mut file, stdout).await?; Ok(()) } @@ -338,6 +339,33 @@ fn validate_read_file_metadata( Ok(()) } +async fn open_read_file_no_follow(path: &Path) -> io::Result { + #[cfg(unix)] + { + tokio::fs::OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOFOLLOW) + .open(path) + .await + } + + #[cfg(windows)] + { + const FILE_FLAG_OPEN_REPARSE_POINT: u32 = 0x0020_0000; + + tokio::fs::OpenOptions::new() + .read(true) + .custom_flags(FILE_FLAG_OPEN_REPARSE_POINT) + .open(path) + .await + } + + #[cfg(not(any(unix, windows)))] + { + tokio::fs::File::open(path).await + } +} + fn map_fs_error(err: io::Error) -> JSONRPCErrorError { match err.kind() { io::ErrorKind::NotFound => not_found(err.to_string()), diff --git a/codex-rs/exec-server/src/fs_sandbox.rs b/codex-rs/exec-server/src/fs_sandbox.rs index 24e9d2e7da..a1c46f29e7 100644 --- a/codex-rs/exec-server/src/fs_sandbox.rs +++ b/codex-rs/exec-server/src/fs_sandbox.rs @@ -198,9 +198,7 @@ fn add_helper_runtime_permissions( } } -pub(crate) fn normalize_file_system_policy_root_aliases( - file_system_policy: &mut FileSystemSandboxPolicy, -) { +fn normalize_file_system_policy_root_aliases(file_system_policy: &mut FileSystemSandboxPolicy) { for entry in &mut file_system_policy.entries { if let FileSystemPath::Path { path } = &mut entry.path { *path = normalize_top_level_alias(path.clone()); diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 520939c14d..0a21f9768b 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -1,7 +1,5 @@ use async_trait::async_trait; -use bytes::Bytes; use codex_utils_absolute_path::AbsolutePathBuf; -use futures::StreamExt; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -301,7 +299,7 @@ impl ExecutorFileSystem for DirectFileSystem { Ok(FileReadBody { file_name: file_name_from_path(path.as_path()), file_size_bytes: metadata.len(), - stream: Box::pin(ReaderStream::new(file).map(|chunk| chunk.map(Bytes::from))), + stream: Box::pin(ReaderStream::new(file)), }) } diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 50a00f2007..a895584e95 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -18,8 +18,6 @@ use crate::RemoveOptions; use crate::fs_helper::FsHelperPayload; use crate::fs_helper::FsHelperRequest; use crate::fs_sandbox::FileSystemSandboxRunner; -use crate::fs_sandbox::normalize_file_system_policy_root_aliases; -use crate::local_file_system::current_sandbox_cwd; use crate::local_file_system::file_name_from_path; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; @@ -86,12 +84,11 @@ impl ExecutorFileSystem for SandboxedFileSystem { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { let sandbox = require_platform_sandbox(sandbox)?; - let resolved_path = resolve_read_path_for_sandbox(path, sandbox).await?; let file_info = self .run_sandboxed( sandbox, FsHelperRequest::ReadFileInfo(FsReadFileParams { - path: resolved_path.clone(), + path: path.clone(), sandbox: None, }), ) @@ -103,7 +100,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { .run_stream( sandbox, FsHelperRequest::ReadFileStream(FsReadFileParams { - path: resolved_path, + path: path.clone(), sandbox: None, }), ) @@ -258,31 +255,6 @@ impl ExecutorFileSystem for SandboxedFileSystem { } } -async fn resolve_read_path_for_sandbox( - path: &AbsolutePathBuf, - sandbox: &FileSystemSandboxContext, -) -> io::Result { - let resolved = tokio::fs::canonicalize(path.as_path()).await?; - let resolved = AbsolutePathBuf::from_absolute_path(resolved)?; - let cwd = match sandbox.cwd.as_ref() { - Some(cwd) => cwd.clone(), - None => AbsolutePathBuf::from_absolute_path(current_sandbox_cwd()?)?, - }; - let mut file_system_policy = sandbox.permissions.file_system_sandbox_policy(); - normalize_file_system_policy_root_aliases(&mut file_system_policy); - if !file_system_policy.can_read_path_with_cwd(resolved.as_path(), cwd.as_path()) { - return Err(io::Error::new( - io::ErrorKind::PermissionDenied, - format!( - "reading `{}` resolved to `{}` which is not permitted by the filesystem sandbox policy", - path.display(), - resolved.display() - ), - )); - } - Ok(resolved) -} - fn require_platform_sandbox( sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<&FileSystemSandboxContext> { diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index 0f27e2be06..6ca1772e51 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -160,6 +160,16 @@ fn assert_sandbox_denied(error: &std::io::Error) { } } +fn assert_read_body_symlink_rejected(error: &std::io::Error) { + if error.kind() == std::io::ErrorKind::InvalidInput + && error.to_string().contains("is not a file") + { + return; + } + + assert_sandbox_denied(error); +} + fn assert_normalized_path_rejected(error: &std::io::Error) { match error.kind() { std::io::ErrorKind::NotFound => assert!( @@ -707,7 +717,7 @@ async fn file_system_sandboxed_read_body_rejects_symlink_to_denied_file( Ok(_) => anyhow::bail!("read body should be blocked"), Err(error) => error, }; - assert_sandbox_denied(&error); + assert_read_body_symlink_rejected(&error); Ok(()) }