This commit is contained in:
won
2026-06-10 11:59:07 -07:00
parent 8a299fb704
commit 401059df75
8 changed files with 144 additions and 5 deletions
@@ -1,15 +1,20 @@
use std::sync::Arc;
use std::sync::Weak;
use codex_exec_server::ExecutorFileSystem;
use codex_exec_server::FileSystemSandboxContext;
use codex_protocol::items::TurnItem;
use codex_tools::ConversationHistory;
use codex_tools::ExtensionTurnItem;
use codex_tools::ToolCall as ExtensionToolCall;
use codex_tools::ToolFileReadFuture;
use codex_tools::ToolFileReader;
use codex_tools::ToolName;
use codex_tools::ToolSearchInfo;
use codex_tools::ToolSpec;
use codex_tools::TurnItemEmissionFuture;
use codex_tools::TurnItemEmitter;
use codex_utils_absolute_path::AbsolutePathBuf;
use crate::function_tool::FunctionCallError;
use crate::session::session::Session;
@@ -71,6 +76,17 @@ struct CoreTurnItemEmitter {
turn: Weak<TurnContext>,
}
struct CoreToolFileReader {
file_system: Arc<dyn ExecutorFileSystem>,
sandbox: FileSystemSandboxContext,
}
impl ToolFileReader for CoreToolFileReader {
fn read_file<'a>(&'a self, path: &'a AbsolutePathBuf) -> ToolFileReadFuture<'a> {
self.file_system.read_file(path, Some(&self.sandbox))
}
}
fn extension_turn_item(item: ExtensionTurnItem) -> TurnItem {
match item {
ExtensionTurnItem::WebSearch(item) => TurnItem::WebSearch(item),
@@ -115,6 +131,15 @@ impl TurnItemEmitter for CoreTurnItemEmitter {
async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
let conversation_history =
ConversationHistory::new(invocation.session.clone_history().await.into_raw_items());
let file_reader = invocation.turn.environments.primary().map(|environment| {
Arc::new(CoreToolFileReader {
file_system: environment.environment.get_filesystem(),
sandbox: invocation.turn.file_system_sandbox_context(
/*additional_permissions*/ None,
&environment.cwd,
),
}) as Arc<dyn ToolFileReader>
});
ExtensionToolCall {
turn_id: invocation.turn.sub_id.clone(),
call_id: invocation.call_id.clone(),
@@ -126,6 +151,7 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
session: Arc::downgrade(&invocation.session),
turn: Arc::downgrade(&invocation.turn),
}),
file_reader,
payload: invocation.payload.clone(),
}
}
@@ -139,16 +165,24 @@ mod tests {
use codex_protocol::items::TurnItem;
use codex_protocol::items::WebSearchItem;
use codex_protocol::models::ContentItem;
use codex_protocol::models::PermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_protocol::models::WebSearchAction;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::EventMsg;
use codex_tools::ExtensionTurnItem;
use codex_tools::ToolFileReader as _;
use codex_utils_absolute_path::test_support::PathExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use serde_json::json;
use tokio::sync::Mutex;
use super::CoreToolFileReader;
use super::CoreTurnItemEmitter;
use super::ExtensionToolAdapter;
use crate::tools::context::ToolCallSource;
@@ -275,6 +309,42 @@ mod tests {
);
}
#[tokio::test]
async fn file_system_capability_enforces_turn_sandbox() {
let (_session, mut turn) = crate::session::tests::make_session_and_context().await;
let workspace = tempfile::tempdir().expect("create temp directory");
let cwd = workspace.path().abs();
let denied_path = cwd.join("denied.png");
std::fs::write(&denied_path, b"image bytes").expect("write denied file");
turn.environments.turn_environments[0].cwd = cwd.clone();
let mut file_system_policy = FileSystemSandboxPolicy::default();
file_system_policy.entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: denied_path.clone(),
},
access: FileSystemAccessMode::Deny,
});
turn.permission_profile = PermissionProfile::from_runtime_permissions(
&file_system_policy,
NetworkSandboxPolicy::Restricted,
);
let environment = turn.environments.primary().expect("primary environment");
let file_reader = CoreToolFileReader {
file_system: environment.environment.get_filesystem(),
sandbox: turn.file_system_sandbox_context(
/*additional_permissions*/ None,
&environment.cwd,
),
};
file_reader
.read_file(&denied_path)
.await
.expect_err("sandbox should reject denied file");
}
#[tokio::test]
async fn passes_turn_fields_and_scoped_turn_item_emitter_to_extension_call() {
let captured_call = Arc::new(Mutex::new(None));
@@ -331,6 +401,7 @@ mod tests {
);
assert_eq!(captured_call.model, model);
assert_eq!(captured_call.truncation_policy, truncation_policy);
assert!(captured_call.file_reader.is_some());
assert_eq!(
captured_call.conversation_history.items(),
std::slice::from_ref(&history_item)
+2
View File
@@ -20,6 +20,8 @@ pub use codex_tools::NoopTurnItemEmitter;
pub use codex_tools::ResponsesApiTool;
pub use codex_tools::ToolCall;
pub use codex_tools::ToolExecutor;
pub use codex_tools::ToolFileReadFuture;
pub use codex_tools::ToolFileReader;
pub use codex_tools::ToolName;
pub use codex_tools::ToolOutput;
pub use codex_tools::ToolPayload;
@@ -1317,6 +1317,7 @@ fn tool_call(tool_name: &str, call_id: &str, arguments: serde_json::Value) -> To
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload: ToolPayload::Function {
arguments: arguments.to_string(),
},
+6 -1
View File
@@ -46,7 +46,8 @@ fn omitted_references_generate_with_fixed_defaults() {
referenced_image_paths: None,
num_last_images_to_include: None,
},
&[]
&[],
/*referenced_images*/ None,
)
.expect("generation request should build"),
ImageRequest::Generate(ImageGenerationRequest {
@@ -118,6 +119,7 @@ fn recent_image_fallback_selects_newest_images_in_chronological_order() {
num_last_images_to_include: Some(4),
},
&history,
/*referenced_images*/ None,
)
.expect("history-backed edit request should build"),
ImageRequest::Edit(expected_edit_request(
@@ -140,6 +142,7 @@ fn conflicting_image_selectors_return_tool_error() {
num_last_images_to_include: Some(1),
},
&[],
/*referenced_images*/ None,
)
.expect_err("conflicting selectors should fail");
@@ -166,6 +169,7 @@ fn too_many_referenced_image_paths_return_tool_error() {
num_last_images_to_include: None,
},
&[],
/*referenced_images*/ None,
)
.expect_err("too many paths should fail before reading files");
@@ -189,6 +193,7 @@ fn recent_image_fallback_requires_requested_count() {
content: vec![input_image("only-image")],
phase: None,
}],
/*referenced_images*/ None,
)
.expect_err("history-backed edit should require the requested image count");
+40 -4
View File
@@ -11,6 +11,7 @@ use codex_extension_api::ExtensionTurnItem;
use codex_extension_api::FunctionCallError;
use codex_extension_api::ToolCall;
use codex_extension_api::ToolExecutor;
use codex_extension_api::ToolFileReader;
use codex_extension_api::ToolName;
use codex_extension_api::ToolOutput;
use codex_extension_api::ToolPayload;
@@ -98,7 +99,9 @@ impl ToolExecutor<ToolCall> for ImageGenerationTool {
/// Executes the selected image operation and returns the completed image result.
async fn handle(&self, call: ToolCall) -> Result<Box<dyn ToolOutput>, FunctionCallError> {
let args = parse_args(&call)?;
let request = request_for_args(&args, call.conversation_history.items())?;
let referenced_images = load_referenced_images(&args, call.file_reader.as_deref()).await?;
let request =
request_for_args(&args, call.conversation_history.items(), referenced_images)?;
call.turn_item_emitter
.emit_started(ExtensionTurnItem::ImageGeneration(ImageGenerationItem {
id: call.call_id.clone(),
@@ -149,10 +152,36 @@ enum ImageRequest {
Edit(ImageEditRequest),
}
async fn load_referenced_images(
args: &ImagegenArgs,
file_reader: Option<&dyn ToolFileReader>,
) -> Result<Option<Vec<ImageUrl>>, FunctionCallError> {
let paths = args.referenced_image_paths.as_deref().unwrap_or_default();
if paths.is_empty()
|| args.num_last_images_to_include.is_some()
|| paths.len() > MAX_EDIT_IMAGES
{
return Ok(None);
}
let Some(file_reader) = file_reader else {
return Err(FunctionCallError::RespondToModel(
"referenced image paths are unavailable in this session".to_string(),
));
};
let mut referenced_images = Vec::with_capacity(paths.len());
for path in paths {
referenced_images.push(image_url(path, file_reader).await?);
}
Ok(Some(referenced_images))
}
/// Builds a generation or edit request from the mutually exclusive image selectors.
fn request_for_args(
args: &ImagegenArgs,
history: &[ResponseItem],
referenced_images: Option<Vec<ImageUrl>>,
) -> Result<ImageRequest, FunctionCallError> {
let paths = args.referenced_image_paths.as_deref().unwrap_or_default();
if paths.len() > MAX_EDIT_IMAGES {
@@ -171,7 +200,11 @@ fn request_for_args(
size: Some("auto".to_string()),
}));
}
(false, None) => paths.iter().map(image_url).collect::<Result<Vec<_>, _>>()?,
(false, None) => referenced_images.ok_or_else(|| {
FunctionCallError::RespondToModel(
"referenced image paths are unavailable in this session".to_string(),
)
})?,
(true, Some(count)) => {
if !(1..=MAX_EDIT_IMAGES).contains(&count) {
return Err(FunctionCallError::RespondToModel(format!(
@@ -302,8 +335,11 @@ fn output_image_urls(output: &FunctionCallOutputPayload) -> impl Iterator<Item =
})
}
fn image_url(path: &AbsolutePathBuf) -> Result<ImageUrl, FunctionCallError> {
let bytes = std::fs::read(path).map_err(|error| {
async fn image_url(
path: &AbsolutePathBuf,
file_reader: &dyn ToolFileReader,
) -> Result<ImageUrl, FunctionCallError> {
let bytes = file_reader.read_file(path).await.map_err(|error| {
FunctionCallError::RespondToModel(format!(
"unable to read referenced image at `{}`: {error}",
path.display()
+6
View File
@@ -215,6 +215,7 @@ async fn add_ad_hoc_note_tool_creates_note_file() {
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload: payload.clone(),
})
.await
@@ -258,6 +259,7 @@ async fn add_ad_hoc_note_tool_rejects_paths_as_filenames() {
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload,
})
.await;
@@ -302,6 +304,7 @@ async fn read_tool_reads_memory_file() {
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload: payload.clone(),
})
.await
@@ -349,6 +352,7 @@ async fn search_tool_accepts_multiple_queries() {
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload: payload.clone(),
})
.await
@@ -422,6 +426,7 @@ async fn search_tool_accepts_windowed_all_match_mode() {
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload: payload.clone(),
})
.await
@@ -475,6 +480,7 @@ async fn search_tool_rejects_legacy_single_query() {
truncation_policy: TruncationPolicy::Bytes(1024),
conversation_history: codex_extension_api::ConversationHistory::default(),
turn_item_emitter: Arc::new(NoopTurnItemEmitter),
file_reader: None,
payload,
})
.await;
+2
View File
@@ -66,6 +66,8 @@ pub use tool_call::ConversationHistory;
pub use tool_call::ExtensionTurnItem;
pub use tool_call::NoopTurnItemEmitter;
pub use tool_call::ToolCall;
pub use tool_call::ToolFileReadFuture;
pub use tool_call::ToolFileReader;
pub use tool_call::TurnItemEmissionFuture;
pub use tool_call::TurnItemEmitter;
pub use tool_config::ShellCommandBackendConfig;
+16
View File
@@ -4,8 +4,10 @@ use crate::ToolPayload;
use codex_protocol::items::ImageGenerationItem;
use codex_protocol::items::WebSearchItem;
use codex_protocol::models::ResponseItem;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_output_truncation::TruncationPolicy;
use std::future::Future;
use std::io;
use std::pin::Pin;
use std::sync::Arc;
@@ -63,6 +65,15 @@ impl TurnItemEmitter for NoopTurnItemEmitter {
}
}
/// Future returned when an extension tool reads a file through the host.
pub type ToolFileReadFuture<'a> = Pin<Box<dyn Future<Output = io::Result<Vec<u8>>> + Send + 'a>>;
/// Host-provided sandboxed file reads for extension tools.
pub trait ToolFileReader: Send + Sync {
/// Reads one file through host-owned sandbox enforcement.
fn read_file<'a>(&'a self, path: &'a AbsolutePathBuf) -> ToolFileReadFuture<'a>;
}
#[derive(Clone)]
pub struct ToolCall {
pub turn_id: String,
@@ -72,6 +83,7 @@ pub struct ToolCall {
pub truncation_policy: TruncationPolicy,
pub conversation_history: ConversationHistory,
pub turn_item_emitter: Arc<dyn TurnItemEmitter>,
pub file_reader: Option<Arc<dyn ToolFileReader>>,
pub payload: ToolPayload,
}
@@ -85,6 +97,10 @@ impl std::fmt::Debug for ToolCall {
.field("truncation_policy", &self.truncation_policy)
.field("conversation_history", &self.conversation_history)
.field("turn_item_emitter", &"<host turn item emitter>")
.field(
"file_reader",
&self.file_reader.as_ref().map(|_| "<host file reader>"),
)
.field("payload", &self.payload)
.finish()
}