mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Apply sandbox context to local view_image reads (#21861)
## Summary - create a selected-cwd filesystem sandbox context for view_image metadata and file reads in both local and remote environments - add a local restricted-profile regression test for the previously unsandboxed read path ## Validation - just fmt - bazel test --bes_backend= --bes_results_url= --test_output=errors --test_filter=view_image::tests::handle_passes_sandbox_context_for_local_filesystem_reads //codex-rs/core:core-unit-tests --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
fd24c00b0b
commit
17ed5ad0b0
@@ -133,16 +133,12 @@ impl ToolHandler for ViewImageHandler {
|
||||
};
|
||||
let cwd = turn_environment.cwd.clone();
|
||||
let abs_path = cwd.join(path);
|
||||
let sandbox = turn_environment.environment.is_remote().then(|| {
|
||||
let mut sandbox =
|
||||
turn.file_system_sandbox_context(/*additional_permissions*/ None);
|
||||
sandbox.cwd = Some(cwd.clone());
|
||||
sandbox
|
||||
});
|
||||
let mut sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None);
|
||||
sandbox.cwd = Some(cwd.clone());
|
||||
let fs = turn_environment.environment.get_filesystem();
|
||||
|
||||
let metadata = fs
|
||||
.get_metadata(&abs_path, sandbox.as_ref())
|
||||
.get_metadata(&abs_path, Some(&sandbox))
|
||||
.await
|
||||
.map_err(|error| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
@@ -158,7 +154,7 @@ impl ToolHandler for ViewImageHandler {
|
||||
)));
|
||||
}
|
||||
let file_bytes = fs
|
||||
.read_file(&abs_path, sandbox.as_ref())
|
||||
.read_file(&abs_path, Some(&sandbox))
|
||||
.await
|
||||
.map_err(|error| {
|
||||
FunctionCallError::RespondToModel(format!(
|
||||
@@ -247,8 +243,15 @@ impl ToolOutput for ViewImageOutput {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::session::tests::make_session_and_context;
|
||||
use crate::tools::context::ToolCallSource;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::turn_diff_tracker::TurnDiffTracker;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::Mutex;
|
||||
|
||||
#[test]
|
||||
fn code_mode_result_returns_image_url_object() {
|
||||
@@ -269,4 +272,35 @@ mod tests {
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn handle_passes_sandbox_context_for_local_filesystem_reads() {
|
||||
let (session, mut turn) = make_session_and_context().await;
|
||||
let image_path = turn.cwd.join("image.png");
|
||||
std::fs::write(image_path.as_path(), b"not a real image").expect("write test image");
|
||||
turn.permission_profile = PermissionProfile::read_only();
|
||||
|
||||
let result = ViewImageHandler::default()
|
||||
.handle(ToolInvocation {
|
||||
session: Arc::new(session),
|
||||
turn: Arc::new(turn),
|
||||
cancellation_token: tokio_util::sync::CancellationToken::new(),
|
||||
tracker: Arc::new(Mutex::new(TurnDiffTracker::new())),
|
||||
call_id: "call-view-image".to_string(),
|
||||
tool_name: codex_tools::ToolName::plain("view_image"),
|
||||
source: ToolCallSource::Direct,
|
||||
payload: ToolPayload::Function {
|
||||
arguments: json!({ "path": "image.png" }).to_string(),
|
||||
},
|
||||
})
|
||||
.await;
|
||||
|
||||
let Err(FunctionCallError::RespondToModel(message)) = result else {
|
||||
panic!("expected sandboxed filesystem error");
|
||||
};
|
||||
assert!(
|
||||
message.contains("sandboxed filesystem operations require configured runtime paths"),
|
||||
"{message}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,6 +18,11 @@ use codex_protocol::openai_models::ModelsResponse;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
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::AskForApproval;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
@@ -472,6 +477,82 @@ async fn view_image_routes_to_selected_local_environment() -> anyhow::Result<()>
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn view_image_tool_applies_local_sandbox_read_denies() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex();
|
||||
let test = builder.build(&server).await?;
|
||||
let rel_path = "denied.png";
|
||||
let denied_path = test.config.cwd.join(rel_path);
|
||||
write_workspace_file(
|
||||
&test,
|
||||
rel_path,
|
||||
png_bytes(/*width*/ 1, /*height*/ 1, [0, 255, 0, 255])?,
|
||||
)
|
||||
.await?;
|
||||
let call_id = "call-view-image-outside-cwd";
|
||||
let response_mock = mount_sse_sequence(
|
||||
&server,
|
||||
vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(
|
||||
call_id,
|
||||
"view_image",
|
||||
&json!({ "path": rel_path }).to_string(),
|
||||
),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut file_system_sandbox_policy = FileSystemSandboxPolicy::default();
|
||||
file_system_sandbox_policy
|
||||
.entries
|
||||
.push(FileSystemSandboxEntry {
|
||||
path: FileSystemPath::Path {
|
||||
path: denied_path.clone(),
|
||||
},
|
||||
access: FileSystemAccessMode::None,
|
||||
});
|
||||
let permission_profile = PermissionProfile::from_runtime_permissions(
|
||||
&file_system_sandbox_policy,
|
||||
NetworkSandboxPolicy::Restricted,
|
||||
);
|
||||
|
||||
test.submit_turn_with_permission_profile("attach the denied image", permission_profile)
|
||||
.await?;
|
||||
|
||||
let request = response_mock
|
||||
.last_request()
|
||||
.context("missing request containing sandboxed view_image output")?;
|
||||
assert!(
|
||||
request.inputs_of_type("input_image").is_empty(),
|
||||
"sandboxed local view_image should not attach denied images"
|
||||
);
|
||||
let output_text = request
|
||||
.function_call_output_content_and_success(call_id)
|
||||
.and_then(|(content, _)| content)
|
||||
.context("sandboxed view_image error text present")?;
|
||||
let expected_locate_prefix = format!("unable to locate image at `{}`:", denied_path.display());
|
||||
let expected_read_prefix = format!("unable to read image at `{}`:", denied_path.display());
|
||||
assert!(
|
||||
output_text.starts_with(&expected_locate_prefix)
|
||||
|| output_text.starts_with(&expected_read_prefix),
|
||||
"expected error to start with `{expected_locate_prefix}` or `{expected_read_prefix}` but got `{output_text}`"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
Reference in New Issue
Block a user