agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

[codex] reject remote images at app-server ingress (#29419)

Browse Source 
## Stack

Stacked on #29417. Review and land that PR first.

## Summary

- reject HTTP(S) image URLs in the handlers for `turn/start` and
`turn/steer`
- validate `thread/inject_items` after its existing
JSON-to-`ResponseItem` conversion, so each item is deserialized once
- turn invalid dynamic-tool image responses into the existing
unsuccessful text fallback; the model receives the validation message as
the function output
- leave `thread/resume.history` compatible with legacy history; #29417
replaces remote images before model input
- continue accepting inline data URLs and `localImage` inputs
- keep this policy in app-server; this PR does not add a shared protocol
API or change core image preparation

## Test plan

- `just test -p codex-app-server -E
'test(/request_handlers_reject_remote_image_urls|dynamic_tool_remote_image_response_becomes_model_visible_error|dynamic_tool_call_round_trip_sends_content_items_to_model|turn_start_tracks_turn_event_analytics|standalone_image_edit_uses_recent_pathless_image/)'`
(5 passed)
- `just fix -p codex-app-server`
- `just fmt`
This commit is contained in:
rka-oai
2026-06-22 17:43:56 -07:00
committed by GitHub
Unverified
parent fbd575ab4a
commit b294638bb5
9 changed files with 296 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/README.md
+5 -3
View File
@@ -714,9 +714,11 @@ If the thread does not already have an active turn, the server starts a standalo
Turns attach user input (text or images) to a thread and trigger Codex generation. The `input` field is a list of discriminated unions:
- `{"type":"text","text":"Explain this diff"}`
- `{"type":"image","url":"https://…png"}`
- `{"type":"image","url":"data:image/png;base64,…"}`
- `{"type":"localImage","path":"/tmp/screenshot.png"}`
The `image` variant accepts inline data URLs. Remote HTTP(S) image URLs are rejected; use a data URL or `localImage` instead.
You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment ids to override the sticky selection for this turn only.
`approvalsReviewer` accepts:
@@ -828,7 +830,7 @@ Invoke a plugin by including a UI mention token such as `@sample` in the text in
### Example: Inject raw history items
Use `thread/inject_items` to append prebuilt Responses API items to a loaded thread’s prompt history without starting a user turn. These items are persisted to the rollout and included in subsequent model requests.
Use `thread/inject_items` to append prebuilt Responses API items to a loaded thread’s prompt history without starting a user turn. These items are persisted to the rollout and included in subsequent model requests. Any `input_image` items must use inline data URLs; remote HTTP(S) image URLs are rejected.
```json
{ "method": "thread/inject_items", "id": 36, "params": {
@@ -1585,7 +1587,7 @@ The server also emits item lifecycle notifications around the request:
3. Client response.
4. `item/completed` with `item.type = "dynamicToolCall"`, final `status`, and the returned `contentItems`/`success`.
The client must respond with content items. Use `inputText` for text and `inputImage` for image URLs/data URLs:
The client must respond with content items. Use `inputText` for text and `inputImage` for inline data URLs. Remote HTTP(S) image URLs make the dynamic tool response invalid.
```json
{
codex-rs/app-server/src/dynamic_tools.rs
+17
View File
@@ -8,6 +8,8 @@ use std::sync::Arc;
use tokio::sync::oneshot;
use tracing::error;
use crate::image_url::REMOTE_IMAGE_URL_ERROR;
use crate::image_url::is_remote_image_url;
use crate::outgoing_message::ClientRequestResult;
use crate::server_request_error::is_turn_transition_server_request_error;
@@ -54,6 +56,21 @@ pub(crate) async fn on_call_response(
fn decode_response(value: serde_json::Value) -> (DynamicToolCallResponse, Option<String>) {
match serde_json::from_value::<DynamicToolCallResponse>(value) {
Ok(response)
if response.content_items.iter().any(|item| {
matches!(
item,
DynamicToolCallOutputContentItem::InputImage { image_url }
if is_remote_image_url(image_url)
)
}) =>
{
error!(
message = REMOTE_IMAGE_URL_ERROR,
"dynamic tool response was invalid"
);
fallback_response(REMOTE_IMAGE_URL_ERROR)
}
Ok(response) => (response, None),
Err(err) => {
error!("failed to deserialize DynamicToolCallResponse: {err}");
codex-rs/app-server/src/image_url.rs
+8
View File
@@ -0,0 +1,8 @@
pub(crate) const REMOTE_IMAGE_URL_ERROR: &str =
"remote image URLs are not supported; use an inline data URL instead";
pub(crate) fn is_remote_image_url(image_url: &str) -> bool {
image_url.split_once(':').is_some_and(|(scheme, _)| {
scheme.eq_ignore_ascii_case("http") || scheme.eq_ignore_ascii_case("https")
})
}
codex-rs/app-server/src/lib.rs
+1
View File
@@ -96,6 +96,7 @@ mod extensions;
mod filters;
mod fs_watch;
mod fuzzy_file_search;
mod image_url;
pub mod in_process;
mod mcp_refresh;
mod message_processor;
codex-rs/app-server/src/request_processors/turn_processor.rs
+59
View File
@@ -1,14 +1,70 @@
use super::*;
use codex_protocol::config_types::MultiAgentMode;
use codex_protocol::models::ContentItem;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::protocol::AdditionalContextEntry as CoreAdditionalContextEntry;
use codex_protocol::protocol::AdditionalContextKind as CoreAdditionalContextKind;
use codex_protocol::protocol::MultiAgentVersion;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use crate::image_url::REMOTE_IMAGE_URL_ERROR;
use crate::image_url::is_remote_image_url;
const DIRECT_INPUT_TO_MULTI_AGENT_V2_SUBAGENT_ERROR: &str =
"direct app-server input is not allowed for multi-agent v2 sub-agents";
fn validate_user_input_image_urls(input: &[V2UserInput]) -> Result<(), JSONRPCErrorError> {
if input.iter().any(|item| {
matches!(
item,
V2UserInput::Image { url, .. } if is_remote_image_url(url)
)
}) {
return Err(invalid_request(REMOTE_IMAGE_URL_ERROR));
}
Ok(())
}
fn validate_response_item_image_urls(items: &[ResponseItem]) -> Result<(), JSONRPCErrorError> {
if items.iter().any(|item| match item {
ResponseItem::Message { content, .. } => content.iter().any(|item| {
matches!(
item,
ContentItem::InputImage { image_url, .. } if is_remote_image_url(image_url)
)
}),
ResponseItem::FunctionCallOutput { output, .. }
| ResponseItem::CustomToolCallOutput { output, .. } => {
output.content_items().is_some_and(|content| {
content.iter().any(|item| {
matches!(
item,
FunctionCallOutputContentItem::InputImage { image_url, .. }
if is_remote_image_url(image_url)
)
})
})
}
ResponseItem::Reasoning { .. }
| ResponseItem::AgentMessage { .. }
| ResponseItem::LocalShellCall { .. }
| ResponseItem::FunctionCall { .. }
| ResponseItem::ToolSearchCall { .. }
| ResponseItem::CustomToolCall { .. }
| ResponseItem::ToolSearchOutput { .. }
| ResponseItem::WebSearchCall { .. }
| ResponseItem::ImageGenerationCall { .. }
| ResponseItem::Compaction { .. }
| ResponseItem::CompactionTrigger { .. }
| ResponseItem::ContextCompaction { .. }
| ResponseItem::Other => false,
}) {
return Err(invalid_request(REMOTE_IMAGE_URL_ERROR));
}
Ok(())
}
#[derive(Clone)]
pub(crate) struct TurnRequestProcessor {
auth_manager: Arc<AuthManager>,
@@ -105,6 +161,7 @@ impl TurnRequestProcessor {
app_server_client_version: Option<String>,
supports_openai_form_elicitation: bool,
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
validate_user_input_image_urls(&params.input)?;
self.turn_start_inner(
request_id,
params,
@@ -140,6 +197,7 @@ impl TurnRequestProcessor {
request_id: &ConnectionRequestId,
params: TurnSteerParams,
) -> Result<Option<ClientResponsePayload>, JSONRPCErrorError> {
validate_user_input_image_urls(&params.input)?;
self.turn_steer_inner(request_id, params)
.await
.map(|response| Some(response.into()))
@@ -764,6 +822,7 @@ impl TurnRequestProcessor {
})
.collect::<std::result::Result<Vec<_>, _>>()
.map_err(invalid_request)?;
validate_response_item_image_urls(&items)?;
thread
.inject_response_items(items)
codex-rs/app-server/tests/suite/v2/dynamic_tools.rs
+93 -12
View File
@@ -39,6 +39,8 @@ use tokio::time::timeout;
use wiremock::MockServer;
const TINY_PNG_DATA_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
const REMOTE_IMAGE_URL_ERROR: &str =
"remote image URLs are not supported; use an inline data URL instead";
// macOS and Windows Bazel CI can spend tens of seconds starting app-server
// subprocesses or processing test RPCs under load.
@@ -552,15 +554,19 @@ async fn dynamic_tool_call_round_trip_sends_text_content_items_to_model() -> Res
Ok(())
}
/// Ensures dynamic tool call responses can include structured content items.
#[tokio::test]
async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<()> {
let call_id = "dyn-call-items-1";
struct PendingDynamicToolCall {
mcp: TestAppServer,
server: MockServer,
request_id: RequestId,
params: DynamicToolCallParams,
}
async fn start_function_dynamic_tool_call(call_id: &str) -> Result<PendingDynamicToolCall> {
let tool_name = "demo_tool";
let tool_args = json!({ "city": "Paris" });
let tool_call_arguments = serde_json::to_string(&tool_args)?;
let responses = vec![
let response_sequence = vec![
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call(call_id, tool_name, &tool_call_arguments),
@@ -568,7 +574,7 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
]),
create_final_assistant_message_sse_response("Done")?,
];
let server = create_mock_responses_server_sequence_unchecked(responses).await;
let server = create_mock_responses_server_sequence_unchecked(response_sequence).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri())?;
@@ -632,20 +638,39 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
mcp.read_stream_until_request_message(),
)
.await??;
let (request_id, params) = match request {
let (request_id, actual_params) = match request {
ServerRequest::DynamicToolCall { request_id, params } => (request_id, params),
other => panic!("expected DynamicToolCall request, got {other:?}"),
};
let expected = DynamicToolCallParams {
let params = DynamicToolCallParams {
thread_id,
turn_id: turn_id.clone(),
turn_id,
call_id: call_id.to_string(),
namespace: None,
tool: tool_name.to_string(),
arguments: tool_args,
};
assert_eq!(params, expected);
assert_eq!(actual_params, params);
Ok(PendingDynamicToolCall {
mcp,
server,
request_id,
params,
})
}
/// Ensures dynamic tool call responses can include structured content items.
#[tokio::test]
async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<()> {
let call_id = "dyn-call-items-1";
let PendingDynamicToolCall {
mut mcp,
server,
request_id,
params,
} = start_function_dynamic_tool_call(call_id).await?;
let response_content_items = vec![
DynamicToolCallOutputContentItem::InputText {
@@ -678,8 +703,8 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
.await?;
let completed = wait_for_dynamic_tool_completed(&mut mcp, call_id).await?;
assert_eq!(completed.thread_id, expected.thread_id.clone());
assert_eq!(completed.turn_id, turn_id);
assert_eq!(completed.thread_id, params.thread_id);
assert_eq!(completed.turn_id, params.turn_id);
let ThreadItem::DynamicToolCall {
status,
content_items: completed_content_items,
@@ -746,6 +771,62 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<(
Ok(())
}
#[tokio::test]
async fn dynamic_tool_remote_image_response_becomes_model_visible_error() -> Result<()> {
let call_id = "dyn-call-remote-image";
let PendingDynamicToolCall {
mut mcp,
server,
request_id,
params,
} = start_function_dynamic_tool_call(call_id).await?;
let response = DynamicToolCallResponse {
content_items: vec![DynamicToolCallOutputContentItem::InputImage {
image_url: "https://example.com/tool.png".to_string(),
}],
success: true,
};
mcp.send_response(request_id, serde_json::to_value(response)?)
.await?;
let completed = wait_for_dynamic_tool_completed(&mut mcp, call_id).await?;
assert_eq!(completed.thread_id, params.thread_id);
assert_eq!(completed.turn_id, params.turn_id);
let ThreadItem::DynamicToolCall {
status,
content_items,
success,
..
} = completed.item
else {
panic!("expected dynamic tool call item");
};
assert_eq!(status, DynamicToolCallStatus::Failed);
assert_eq!(
content_items,
Some(vec![DynamicToolCallOutputContentItem::InputText {
text: REMOTE_IMAGE_URL_ERROR.to_string(),
}])
);
assert_eq!(success, Some(false));
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("turn/completed"),
)
.await??;
let output = responses_bodies(&server)
.await?
.iter()
.find_map(|body| function_call_output_raw_output(body, call_id))
.context("expected function_call_output output in follow-up request")?;
assert_eq!(output, json!(REMOTE_IMAGE_URL_ERROR));
Ok(())
}
async fn responses_bodies(server: &MockServer) -> Result<Vec<Value>> {
let requests = server
.received_requests()
codex-rs/app-server/tests/suite/v2/mod.rs
+1
View File
@@ -52,6 +52,7 @@ mod remote_control;
mod remote_thread_store;
mod request_permissions;
mod request_user_input;
mod request_validation;
mod review;
mod safety_check_downgrade;
mod skills_list;
codex-rs/app-server/tests/suite/v2/request_validation.rs
+110
View File
@@ -0,0 +1,110 @@
use anyhow::Result;
use app_test_support::TestAppServer;
use app_test_support::to_response;
use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url;
use codex_app_server_protocol::JSONRPCError;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ThreadStartParams;
use codex_app_server_protocol::ThreadStartResponse;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ImageDetail;
use codex_protocol::models::ResponseItem;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::time::Duration;
use tempfile::TempDir;
use tokio::time::timeout;
const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10);
const REMOTE_IMAGE_URL_ERROR: &str =
"remote image URLs are not supported; use an inline data URL instead";
#[tokio::test]
async fn request_handlers_reject_remote_image_urls() -> Result<()> {
let codex_home = TempDir::new()?;
write_mock_responses_config_toml_with_chatgpt_base_url(
codex_home.path(),
"http://localhost/unused",
"http://localhost/unused",
)?;
let mut mcp = TestAppServer::new(codex_home.path()).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let thread_request_id = mcp
.send_thread_start_request(ThreadStartParams::default())
.await?;
let thread_response: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(thread_request_id)),
)
.await??;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_response)?;
let thread_id = thread.id;
let remote_tool_output = serde_json::to_value(ResponseItem::FunctionCallOutput {
id: None,
call_id: "call-1".to_string(),
output: FunctionCallOutputPayload::from_content_items(vec![
FunctionCallOutputContentItem::InputImage {
image_url: "https://example.com/tool.png".to_string(),
detail: Some(ImageDetail::High),
},
]),
internal_chat_message_metadata_passthrough: None,
})?;
let requests = [
(
"turn/start",
json!({
"threadId": thread_id,
"input": [{
"type": "image",
"url": "HTTP://example.com/start.png",
"detail": "high"
}]
}),
),
(
"turn/steer",
json!({
"threadId": thread_id,
"expectedTurnId": "turn-id",
"input": [{
"type": "image",
"url": "https://example.com/steer.png",
"detail": "high"
}]
}),
),
(
"thread/inject_items",
json!({
"threadId": thread_id,
"items": [remote_tool_output]
}),
),
];
for (method, params) in requests {
let request_id = mcp.send_raw_request(method, Some(params)).await?;
let actual: JSONRPCError = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_error_message(RequestId::Integer(request_id)),
)
.await??;
let expected = JSONRPCError {
id: RequestId::Integer(request_id),
error: JSONRPCErrorError {
code: -32600,
data: None,
message: REMOTE_IMAGE_URL_ERROR.to_string(),
},
};
assert_eq!(actual, expected, "unexpected response for {method}");
}
Ok(())
}
codex-rs/app-server/tests/suite/v2/turn_start.rs
+2 -1
View File
@@ -106,6 +106,7 @@ const TINY_PNG_BYTES: &[u8] = &[
0, 0, 31, 21, 196, 137, 0, 0, 0, 11, 73, 68, 65, 84, 120, 156, 99, 96, 0, 2, 0, 0, 5, 0, 1,
122, 94, 171, 63, 0, 0, 0, 0, 73, 69, 78, 68, 174, 66, 96, 130,
];
const TINY_PNG_DATA_URL: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
fn body_contains(req: &wiremock::Request, text: &str) -> bool {
String::from_utf8(req.body.clone())
@@ -888,7 +889,7 @@ async fn turn_start_tracks_turn_event_analytics() -> Result<()> {
thread_id: thread.id.clone(),
client_user_message_id: None,
input: vec![V2UserInput::Image {
url: "https://example.com/a.png".to_string(),
url: TINY_PNG_DATA_URL.to_string(),
detail: None,
}],
responsesapi_client_metadata: Some(HashMap::from([(