diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index dd0e0e14c1..03adb5a9f1 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -453,22 +453,8 @@ fn validate_dynamic_tools(tools: &[ApiDynamicToolSpec]) -> Result<(), String> { const DYNAMIC_TOOL_NAME_MAX_LEN: usize = 128; const DYNAMIC_TOOL_NAMESPACE_MAX_LEN: usize = 64; const DYNAMIC_TOOL_IDENTIFIER_PATTERN: &str = "^[a-zA-Z0-9_-]+$"; - const RESERVED_RESPONSES_NAMESPACES: &[&str] = &[ - "api_tool", - "browser", - "computer", - "container", - "file_search", - "functions", - "image_gen", - "multi_tool_use", - "python", - "python_user_visible", - "submodel_delegator", - "terminal", - "tool_search", - "web", - ]; + // Single source of truth: codex_tools::RESERVED_RESPONSES_NAMESPACES. + let reserved_responses_namespaces = codex_tools::RESERVED_RESPONSES_NAMESPACES; fn escape_identifier_for_error(value: &str) -> String { value.escape_default().to_string() @@ -537,7 +523,7 @@ fn validate_dynamic_tools(tools: &[ApiDynamicToolSpec]) -> Result<(), String> { "dynamic tool namespace is reserved for {name}: {namespace}" )); } - if RESERVED_RESPONSES_NAMESPACES.contains(&namespace) { + if reserved_responses_namespaces.contains(&namespace) { return Err(format!( "dynamic tool namespace collides with a reserved Responses API namespace for {name}: {namespace}", )); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index ab9d1e58c2..b7fb0b80fb 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -3471,22 +3471,7 @@ fn validate_multi_agent_v2_wait_timeout(label: &str, value: i64) -> std::io::Res fn validate_multi_agent_v2_tool_namespace(namespace: Option<&str>) -> std::io::Result<()> { const LABEL: &str = "features.multi_agent_v2.tool_namespace"; const MAX_LEN: usize = 64; - const RESERVED_RESPONSES_NAMESPACES: &[&str] = &[ - "api_tool", - "browser", - "computer", - "container", - "file_search", - "functions", - "image_gen", - "multi_tool_use", - "python", - "python_user_visible", - "submodel_delegator", - "terminal", - "tool_search", - "web", - ]; + // Single source of truth: codex_tools::RESERVED_RESPONSES_NAMESPACES. let Some(namespace) = namespace else { return Ok(()); @@ -3520,7 +3505,7 @@ fn validate_multi_agent_v2_tool_namespace(namespace: Option<&str>) -> std::io::R } if namespace == "mcp" || namespace.starts_with("mcp__") - || RESERVED_RESPONSES_NAMESPACES.contains(&namespace) + || codex_tools::is_reserved_responses_namespace(namespace) { return Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index a3e98ba0a3..90a0c07336 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -349,11 +349,47 @@ fn build_model_visible_specs_and_registry( .filter(|spec| { namespace_tools_enabled(turn_context) || !matches!(spec, ToolSpec::Namespace(_)) }) + .filter(namespace_spec_is_safe_for_wire) .collect(); (model_visible_specs, registry) } +/// Defense-in-depth guard against a first-party namespace tool being assembled +/// under a Responses-API-reserved built-in namespace. +/// +/// This is the durable regression guard for the `image_gen.imagegen` 400: the +/// standalone image tool must live under the non-reserved `images` namespace, +/// never `image_gen`. If a future refactor (or a second registration path) +/// reintroduces a reserved namespace, this fails loudly in debug/test builds +/// and, in release builds, drops the offending tool so the request still ships +/// (minus one tool) instead of the API rejecting the entire turn. +/// +/// `web` is intentionally allowlisted (see +/// `codex_tools::FIRST_PARTY_ALLOWED_RESERVED_NAMESPACES`): standalone web +/// search advertises the built-in `web.run` schema on purpose. +fn namespace_spec_is_safe_for_wire(spec: &ToolSpec) -> bool { + let ToolSpec::Namespace(namespace) = spec else { + return true; + }; + let forbidden = codex_tools::is_forbidden_first_party_namespace(&namespace.name); + debug_assert!( + !forbidden, + "first-party tool assembled under Responses-API-reserved namespace `{namespace}` \ + (wire name `{namespace}.*`); rename it to a non-reserved namespace, or add it to \ + codex_tools::FIRST_PARTY_ALLOWED_RESERVED_NAMESPACES if it matches the built-in schema", + namespace = namespace.name, + ); + if forbidden { + warn!( + namespace = %namespace.name, + "dropping first-party tool under reserved Responses namespace to avoid a runtime 400", + ); + return false; + } + true +} + fn spec_for_model_request( turn_context: &TurnContext, exposure: ToolExposure, diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index d1d3b84aef..55663fe636 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -2302,3 +2302,101 @@ async fn infinity_agent_policy_removes_host_tools_from_plan() { // no in-binary tool surface whatsoever. infinity.assert_registered_lacks(&["update_plan", "rename_session"]); } + +// --------------------------------------------------------------------------- +// Regression: reserved Responses-API namespace guard (`image_gen.imagegen` 400) +// +// The standalone image tool once registered under the Responses-API-reserved +// `image_gen` namespace (wire name `image_gen.imagegen`), which the model +// rejects at runtime with a 400 that fails the *entire* turn. The tool now +// lives under the non-reserved `images` namespace; these tests pin that the +// assembled Responses-wire tool list can never carry a reserved `image_gen` +// namespace, and that the assembly guard fires if one is ever reintroduced. +// --------------------------------------------------------------------------- + +fn reserved_namespace_spec(namespace: &str) -> ToolSpec { + ToolSpec::Namespace(codex_tools::ResponsesApiNamespace { + name: namespace.to_string(), + description: codex_tools::default_namespace_description(namespace), + tools: vec![ResponsesApiNamespaceTool::Function(ResponsesApiTool { + name: "imagegen".to_string(), + description: "reserved-namespace regression probe".to_string(), + strict: false, + defer_loading: None, + parameters: codex_tools::JsonSchema::default(), + output_schema: None, + })], + }) +} + +#[tokio::test] +async fn standalone_imagegen_never_uses_reserved_image_gen_namespace_on_responses_wire() { + let probe = probe_with( + |turn| { + use_chatgpt_auth(turn); + set_feature(turn, Feature::ImageGeneration, /*enabled*/ true); + set_feature(turn, Feature::ImageGenExt, /*enabled*/ true); + turn.model_info.input_modalities = vec![InputModality::Text, InputModality::Image]; + turn.model_info.use_responses_lite = true; + turn.model_info.tool_mode = Some(ToolMode::Direct); + turn.tool_mode = ToolMode::Direct; + }, + ToolPlanInputs { + extension_tool_executors: vec![Arc::new(ImagegenExtensionTool)], + ..Default::default() + }, + ) + .await; + + // The standalone image tool is present under the non-reserved `images` + // namespace... + let serialized_tools = probe.serialized_tools(); + assert!( + has_serialized_namespace_function(&serialized_tools, "images", "imagegen"), + "standalone imagegen should be exposed as images.imagegen: {serialized_tools:?}" + ); + // ...and NEVER under the Responses-API-reserved `image_gen` namespace. + assert!( + !has_serialized_namespace_function(&serialized_tools, "image_gen", "imagegen"), + "assembled Responses tool list must not contain reserved image_gen.imagegen: {serialized_tools:?}" + ); + // No serialized namespace tool may carry a reserved (non-allowlisted) name. + for tool in &serialized_tools { + if tool.get("type").and_then(Value::as_str) != Some("namespace") { + continue; + } + let name = tool.get("name").and_then(Value::as_str).unwrap_or_default(); + assert!( + !codex_tools::is_forbidden_first_party_namespace(name), + "assembled Responses tool list carries a reserved namespace `{name}`: {serialized_tools:?}" + ); + } + probe.assert_registered_lacks(&["image_genimagegen"]); +} + +#[test] +fn namespace_guard_allows_non_reserved_and_allowlisted_namespaces() { + // Non-reserved first-party namespaces (e.g. the renamed image tool) pass. + assert!(super::namespace_spec_is_safe_for_wire( + &reserved_namespace_spec("images") + )); + // `web` is reserved but explicitly allowlisted (standalone web search). + assert!(super::namespace_spec_is_safe_for_wire( + &reserved_namespace_spec("web") + )); + // Non-namespace specs are never affected by the guard. + assert!(super::namespace_spec_is_safe_for_wire( + &ToolSpec::ImageGeneration { + output_format: "png".to_string(), + } + )); +} + +#[test] +#[should_panic(expected = "reserved")] +fn namespace_guard_fails_loud_on_reserved_image_gen_namespace() { + // In debug/test builds the guard fires a debug_assert so a reintroduced + // reserved namespace (e.g. a second registration path) is caught in CI + // instead of shipping a request the API rejects. + let _ = super::namespace_spec_is_safe_for_wire(&reserved_namespace_spec("image_gen")); +} diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 219278c910..68637d06f4 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -8,6 +8,7 @@ mod image_detail; mod json_schema; mod mcp_tool; mod request_plugin_install; +mod reserved_namespaces; mod response_history; mod responses_api; mod tool_call; @@ -48,6 +49,10 @@ pub use request_plugin_install::RequestPluginInstallResult; pub use request_plugin_install::all_requested_connectors_picked_up; pub use request_plugin_install::build_request_plugin_install_elicitation_request; pub use request_plugin_install::verified_connector_install_completed; +pub use reserved_namespaces::FIRST_PARTY_ALLOWED_RESERVED_NAMESPACES; +pub use reserved_namespaces::RESERVED_RESPONSES_NAMESPACES; +pub use reserved_namespaces::is_forbidden_first_party_namespace; +pub use reserved_namespaces::is_reserved_responses_namespace; pub use response_history::retain_tail_from_last_n_user_messages; pub use response_history::truncate_assistant_output_text_to_token_budget; pub use responses_api::FreeformTool; diff --git a/codex-rs/tools/src/reserved_namespaces.rs b/codex-rs/tools/src/reserved_namespaces.rs new file mode 100644 index 0000000000..3365db5dec --- /dev/null +++ b/codex-rs/tools/src/reserved_namespaces.rs @@ -0,0 +1,117 @@ +//! Namespaces the OpenAI Responses API reserves for its own built-in tools. +//! +//! The Responses API refuses any request whose `tools` array declares a +//! namespaced function under one of these names unless it exactly matches the +//! built-in's configured schema. A custom/first-party tool that collides is +//! rejected at runtime with, e.g.: +//! +//! ```text +//! 400 Function 'image_gen.imagegen' is reserved for use by this model +//! and must match the configured schema. +//! ``` +//! +//! Because that error rejects the *whole* request, a single mis-namespaced tool +//! breaks every turn in the session. To prevent this we (1) reject +//! dynamic/user-supplied tools that try to use a reserved namespace and (2) +//! guard the first-party assembled tool list so a Codewith tool can never be +//! shipped under a reserved namespace it is not schema-compatible with. + +/// Namespaces reserved by the OpenAI Responses API for its built-in tools. +/// +/// Keep this list sorted. It is the single source of truth consumed by the +/// dynamic-tool validator, the `multi_agent_v2` namespace validator, and the +/// first-party tool-assembly guard. +pub const RESERVED_RESPONSES_NAMESPACES: &[&str] = &[ + "api_tool", + "browser", + "computer", + "container", + "file_search", + "functions", + "image_gen", + "multi_tool_use", + "python", + "python_user_visible", + "submodel_delegator", + "terminal", + "tool_search", + "web", +]; + +/// Reserved namespaces that Codewith's own first-party tools are permitted to +/// reuse because the tool is deliberately schema-compatible with the built-in. +/// +/// Currently this is only `web` (standalone web search advertises the built-in +/// `web.run` schema). Every other reserved namespace must never appear as a +/// first-party namespace tool — notably `image_gen`, which the standalone image +/// tool must not use (it lives under the non-reserved `images` namespace). +/// +/// Invariant: if a new first-party tool must legitimately live under a reserved +/// namespace, add that namespace here in the same change that introduces it. +pub const FIRST_PARTY_ALLOWED_RESERVED_NAMESPACES: &[&str] = &["web"]; + +/// True if `namespace` is reserved by the Responses API for a built-in tool. +pub fn is_reserved_responses_namespace(namespace: &str) -> bool { + RESERVED_RESPONSES_NAMESPACES.contains(&namespace) +} + +/// True if a *first-party* namespace tool must not be assembled under +/// `namespace` because it is Responses-API-reserved and not on Codewith's +/// vetted allowlist. +/// +/// This is the regression guard for the `image_gen.imagegen` 400: a +/// first-party / extension / code-mode tool must never be assembled under +/// `image_gen` (or any other reserved namespace except the allowlisted ones). +pub fn is_forbidden_first_party_namespace(namespace: &str) -> bool { + is_reserved_responses_namespace(namespace) + && !FIRST_PARTY_ALLOWED_RESERVED_NAMESPACES.contains(&namespace) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn image_gen_is_reserved_and_forbidden_for_first_party() { + assert!(is_reserved_responses_namespace("image_gen")); + assert!(is_forbidden_first_party_namespace("image_gen")); + } + + #[test] + fn web_is_reserved_but_allowed_for_first_party() { + assert!(is_reserved_responses_namespace("web")); + assert!(!is_forbidden_first_party_namespace("web")); + } + + #[test] + fn non_reserved_namespaces_are_allowed() { + for namespace in [ + "images", + "memory", + "agents", + "multi_agent_v1", + "mcp__server", + ] { + assert!(!is_reserved_responses_namespace(namespace)); + assert!(!is_forbidden_first_party_namespace(namespace)); + } + } + + #[test] + fn reserved_list_is_sorted_and_unique() { + let mut sorted = RESERVED_RESPONSES_NAMESPACES.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!(sorted.as_slice(), RESERVED_RESPONSES_NAMESPACES); + } + + #[test] + fn every_allowlisted_namespace_is_reserved() { + for namespace in FIRST_PARTY_ALLOWED_RESERVED_NAMESPACES { + assert!( + is_reserved_responses_namespace(namespace), + "allowlisting a non-reserved namespace `{namespace}` is meaningless" + ); + } + } +}