clean up redundant old check

This commit is contained in:
colby-oai
2026-04-28 16:26:10 -04:00
parent a8ab08ff2a
commit 5055167b82
7 changed files with 50 additions and 40 deletions
+1
View File
@@ -2621,6 +2621,7 @@ dependencies = [
"codex-utils-pty",
"ctor 0.6.3",
"futures",
"libc",
"pretty_assertions",
"reqwest",
"serde",
+3
View File
@@ -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 }
+31 -3
View File
@@ -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<dyn std::error::Error + Send + Sync>> {
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<tokio::fs::File> {
#[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()),
+1 -3
View File
@@ -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());
@@ -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)),
})
}
@@ -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<FileReadBody> {
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<AbsolutePathBuf> {
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> {
+11 -1
View File
@@ -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(())
}