From 74e112ea09dc5ac4216ece0d31ecde433f32642b Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Tue, 24 Feb 2026 18:11:04 -0800 Subject: [PATCH 01/10] add AWS_LC_SYS_NO_JITTER_ENTROPY=1 to release musl build step to unblock releases (#12720) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit linux musl build steps in `rust-release.yml` are [currently broken](https://github.com/openai/codex/actions/runs/22367312571) because of linking issues due to ubsan-calling types (`jitterentropy`) leaking into the build. add `AWS_LC_SYS_NO_JITTER_ENTROPY=1` to the musl build step to avoid linking those ubsan-calling types. this is a more temporary fix, we need to clean up ubsan usage upstream so they dont leak into release-build steps anyways. codex's more thorough explanation below: [pr 9859](https://github.com/openai/codex/pull/9859) added [MITM init](https://github.com/openai/codex/pull/9859/changes#diff-db782967007060c5520651633e1ea21681d64be21f2b791d3d84519860245b97R62-R68) in network-proxy, which wires in cert generation code (rcgen/rustls). this didnt bump/change dep versions, but it changed symbol reachability at link time. for musl builds, that made aws-lc-sys’s jitterentropy objects get pulled into the final link. those objects contain UBSan calls (__ubsan_handle_*). musl release linking is static (*-linux-musl-gcc, -nodefaultlibs) and does not link a musl UBSan runtime, so link fails with undefined __ubsan_*. before, our custom musl CI UBSan steps (install libubsan1, RUSTC_WRAPPER + LD_PRELOAD, partial flag scrubbing) masked some sanitizer issues. after this pr, more aws-lc code became link-reachable, and that band-aid wasn't enough. --- .github/workflows/rust-release.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/rust-release.yml b/.github/workflows/rust-release.yml index e7fc2c5ef2..4ed08b077e 100644 --- a/.github/workflows/rust-release.yml +++ b/.github/workflows/rust-release.yml @@ -178,6 +178,12 @@ jobs: shell: bash run: | set -euo pipefail + # Avoid problematic aws-lc jitter entropy code path on musl builders. + echo "AWS_LC_SYS_NO_JITTER_ENTROPY=1" >> "$GITHUB_ENV" + target_no_jitter="AWS_LC_SYS_NO_JITTER_ENTROPY_${{ matrix.target }}" + target_no_jitter="${target_no_jitter//-/_}" + echo "${target_no_jitter}=1" >> "$GITHUB_ENV" + # Clear global Rust flags so host/proc-macro builds don't pull in UBSan. echo "RUSTFLAGS=" >> "$GITHUB_ENV" echo "CARGO_ENCODED_RUSTFLAGS=" >> "$GITHUB_ENV" From 125fbec317c41193c3718e21e479c581614a967a Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Tue, 24 Feb 2026 18:23:53 -0800 Subject: [PATCH 02/10] Fix js_repl view_image attachments in nested tool calls (#12725) ## Summary - Fix `js_repl` so `await codex.tool("view_image", { path })` actually attaches the image to the active turn when called from inside the JS REPL. - Restore the behavior expected by the existing `js_repl` image-attachment test. - This is a follow-up to [#12553](https://github.com/openai/codex/pull/12553), which changed `view_image` to return structured image content. ## Root Cause - [#12553](https://github.com/openai/codex/pull/12553) changed `view_image` from directly injecting a pending user image message to returning structured `function_call_output` content items. - The nested tool-call bridge inside `js_repl` serialized that tool response back to the JS runtime, but it did not mirror returned image content into the active turn. - As a result, `view_image` appeared to succeed inside `js_repl`, but no `input_image` was actually attached for the outer turn. ## What Changed - Updated the nested tool-call path in `js_repl` to inspect function tool responses for structured content items. - When a nested tool response includes `input_image` content, `js_repl` now injects a corresponding user `Message` into the active turn before returning the raw tool result back to the JS runtime. - Kept the normal JSON result flow intact, so `codex.tool(...)` still returns the original tool output object to JavaScript. ## Why - `js_repl` documentation and tests already assume that `view_image` can be used from inside the REPL to attach generated images to the model. - Without this fix, the nested call path silently dropped that attachment behavior. --- codex-rs/core/src/codex.rs | 23 +- codex-rs/core/src/tools/context.rs | 1 - .../core/src/tools/handlers/multi_agents.rs | 1 - .../core/src/tools/handlers/view_image.rs | 21 -- codex-rs/core/src/tools/js_repl/mod.rs | 205 +++++++++++++++--- codex-rs/core/src/tools/router.rs | 1 - codex-rs/core/tests/suite/view_image.rs | 47 ++-- 7 files changed, 223 insertions(+), 76 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 31cadc947a..d9f383ef63 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -6361,6 +6361,8 @@ use crate::memories::prompts::build_memory_tool_developer_instructions; #[cfg(test)] pub(crate) use tests::make_session_and_context; #[cfg(test)] +pub(crate) use tests::make_session_and_context_with_dynamic_tools_and_rx; +#[cfg(test)] pub(crate) use tests::make_session_and_context_with_rx; #[cfg(test)] pub(crate) use tests::make_session_configuration_for_tests; @@ -8273,9 +8275,9 @@ mod tests { (session, turn_context) } - // Like make_session_and_context, but returns Arc and the event receiver - // so tests can assert on emitted events. - pub(crate) async fn make_session_and_context_with_rx() -> ( + pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( + dynamic_tools: Vec, + ) -> ( Arc, Arc, async_channel::Receiver, @@ -8327,7 +8329,7 @@ mod tests { thread_name: None, original_config_do_not_use: Arc::clone(&config), session_source: SessionSource::Exec, - dynamic_tools: Vec::new(), + dynamic_tools, persist_extended_history: false, }; let per_turn_config = Session::build_per_turn_config(&session_configuration); @@ -8429,6 +8431,16 @@ mod tests { (session, turn_context, rx_event) } + // Like make_session_and_context, but returns Arc and the event receiver + // so tests can assert on emitted events. + pub(crate) async fn make_session_and_context_with_rx() -> ( + Arc, + Arc, + async_channel::Receiver, + ) { + make_session_and_context_with_dynamic_tools_and_rx(Vec::new()).await + } + #[tokio::test] async fn refresh_mcp_servers_is_deferred_until_next_turn() { let (session, turn_context) = make_session_and_context().await; @@ -9273,7 +9285,6 @@ mod tests { }) .to_string(), }, - source: ToolCallSource::Direct, }) .await; @@ -9313,7 +9324,6 @@ mod tests { }) .to_string(), }, - source: ToolCallSource::Direct, }) .await; @@ -9373,7 +9383,6 @@ mod tests { }) .to_string(), }, - source: ToolCallSource::Direct, }) .await; diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index 081d066873..58925622a0 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -30,7 +30,6 @@ pub struct ToolInvocation { pub call_id: String, pub tool_name: String, pub payload: ToolPayload, - pub source: ToolCallSource, } #[derive(Clone, Debug)] diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index 8fbbd0d112..d9916a300a 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -1004,7 +1004,6 @@ mod tests { call_id: "call-1".to_string(), tool_name: tool_name.to_string(), payload, - source: crate::tools::router::ToolCallSource::Direct, } } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index ea05f736e7..9bf03acb00 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -8,7 +8,6 @@ use tokio::fs; use crate::function_tool::FunctionCallError; use crate::protocol::EventMsg; use crate::protocol::ViewImageToolCallEvent; -use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -16,7 +15,6 @@ use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; use codex_protocol::models::ContentItem; -use codex_protocol::models::ResponseInputItem; use codex_protocol::models::local_image_content_items_with_label_number; pub struct ViewImageHandler; @@ -52,7 +50,6 @@ impl ToolHandler for ViewImageHandler { turn, payload, call_id, - source, .. } = invocation; @@ -85,24 +82,6 @@ impl ToolHandler for ViewImageHandler { let event_path = abs_path.clone(); let content = local_image_content_items_with_label_number(&abs_path, None); - if source == ToolCallSource::JsRepl - && content - .iter() - .any(|item| matches!(item, ContentItem::InputImage { .. })) - { - let input_item = ResponseInputItem::Message { - role: "user".to_string(), - content: content.clone(), - }; - if session - .inject_response_items(vec![input_item]) - .await - .is_err() - { - tracing::warn!("view_image could not find an active turn to attach image input"); - } - } - let content = content .into_iter() .map(|item| match item { diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index a5bd81049f..18695ab889 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -9,6 +9,9 @@ use std::sync::Arc; use std::time::Duration; use codex_protocol::ThreadId; +use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputContentItem; +use codex_protocol::models::ResponseInputItem; use serde::Deserialize; use serde::Serialize; use serde_json::Value as JsonValue; @@ -1050,36 +1053,78 @@ impl JsReplManager { } }; + let tool_name = req.tool_name.clone(); let call = crate::tools::router::ToolCall { - tool_name: req.tool_name, + tool_name: tool_name.clone(), call_id: req.id.clone(), payload, }; + let session = Arc::clone(&exec.session); + let turn = Arc::clone(&exec.turn); + let tracker = Arc::clone(&exec.tracker); + match router .dispatch_tool_call( - exec.session, - exec.turn, - exec.tracker, + session.clone(), + turn, + tracker, call, crate::tools::router::ToolCallSource::JsRepl, ) .await { - Ok(response) => match serde_json::to_value(response) { - Ok(value) => RunToolResult { - id: req.id, - ok: true, - response: Some(value), - error: None, - }, - Err(err) => RunToolResult { - id: req.id, - ok: false, - response: None, - error: Some(format!("failed to serialize tool output: {err}")), - }, - }, + Ok(response) => { + if let ResponseInputItem::FunctionCallOutput { output, .. } = &response + && let Some(items) = output.content_items() + { + let mut has_image = false; + let mut content = Vec::with_capacity(items.len()); + for item in items { + match item { + FunctionCallOutputContentItem::InputText { text } => { + content.push(ContentItem::InputText { text: text.clone() }); + } + FunctionCallOutputContentItem::InputImage { image_url } => { + has_image = true; + content.push(ContentItem::InputImage { + image_url: image_url.clone(), + }); + } + } + } + + if has_image + && session + .inject_response_items(vec![ResponseInputItem::Message { + role: "user".to_string(), + content, + }]) + .await + .is_err() + { + warn!( + tool_name = %tool_name, + "js_repl tool call returned image content but there was no active turn to attach it to" + ); + } + } + + match serde_json::to_value(response) { + Ok(value) => RunToolResult { + id: req.id, + ok: true, + response: Some(value), + error: None, + }, + Err(err) => RunToolResult { + id: req.id, + ok: false, + response: None, + error: Some(format!("failed to serialize tool output: {err}")), + }, + } + } Err(err) => RunToolResult { id: req.id, ok: false, @@ -1301,9 +1346,14 @@ pub(crate) fn resolve_node(config_path: Option<&Path>) -> Option { mod tests { use super::*; use crate::codex::make_session_and_context; + use crate::codex::make_session_and_context_with_dynamic_tools_and_rx; use crate::protocol::AskForApproval; + use crate::protocol::EventMsg; use crate::protocol::SandboxPolicy; use crate::turn_diff_tracker::TurnDiffTracker; + use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem; + use codex_protocol::dynamic_tools::DynamicToolResponse; + use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::ContentItem; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::InputModality; @@ -2116,23 +2166,118 @@ console.log(out.output?.body?.text ?? ""); assert!(result.output.contains("function_call_output")); let pending_input = session.get_pending_input().await; - let image_url = pending_input - .iter() - .find_map(|item| match item { - ResponseInputItem::Message { content, .. } => { - content.iter().find_map(|content_item| match content_item { - ContentItem::InputImage { image_url } => Some(image_url.as_str()), - _ => None, - }) - } - _ => None, - }) - .expect("view_image should inject an input_image message for the active turn"); + let [ResponseInputItem::Message { role, content }] = pending_input.as_slice() else { + panic!( + "view_image should inject exactly one pending input message, got {pending_input:?}" + ); + }; + assert_eq!(role, "user"); + let [ContentItem::InputImage { image_url }] = content.as_slice() else { + panic!( + "view_image should inject exactly one input_image content item, got {content:?}" + ); + }; assert!(image_url.starts_with("data:image/png;base64,")); Ok(()) } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn js_repl_can_attach_image_via_dynamic_tool_with_mixed_content() -> anyhow::Result<()> { + if !can_run_js_repl_runtime_tests().await { + return Ok(()); + } + + let (session, turn, rx_event) = + make_session_and_context_with_dynamic_tools_and_rx(vec![DynamicToolSpec { + name: "inline_image".to_string(), + description: "Returns inline text and image content.".to_string(), + input_schema: serde_json::json!({ + "type": "object", + "properties": {}, + "additionalProperties": false + }), + }]) + .await; + if !turn + .model_info + .input_modalities + .contains(&InputModality::Image) + { + return Ok(()); + } + + *session.active_turn.lock().await = Some(crate::state::ActiveTurn::default()); + + let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); + let manager = turn.js_repl.manager().await?; + let code = r#" +const out = await codex.tool("inline_image", {}); +console.log(out.type); +"#; + let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="; + + let session_for_response = Arc::clone(&session); + let response_watcher = async move { + loop { + let event = tokio::time::timeout(Duration::from_secs(2), rx_event.recv()).await??; + if let EventMsg::DynamicToolCallRequest(request) = event.msg { + session_for_response + .notify_dynamic_tool_response( + &request.call_id, + DynamicToolResponse { + content_items: vec![ + DynamicToolCallOutputContentItem::InputText { + text: "inline image note".to_string(), + }, + DynamicToolCallOutputContentItem::InputImage { + image_url: image_url.to_string(), + }, + ], + success: true, + }, + ) + .await; + return Ok::<(), anyhow::Error>(()); + } + } + }; + + let (result, response_watcher_result) = tokio::join!( + manager.execute( + Arc::clone(&session), + Arc::clone(&turn), + tracker, + JsReplArgs { + code: code.to_string(), + timeout_ms: Some(15_000), + }, + ), + response_watcher, + ); + response_watcher_result?; + let result = result?; + assert!(result.output.contains("function_call_output")); + + let pending_input = session.get_pending_input().await; + assert_eq!( + pending_input, + vec![ResponseInputItem::Message { + role: "user".to_string(), + content: vec![ + ContentItem::InputText { + text: "inline image note".to_string(), + }, + ContentItem::InputImage { + image_url: image_url.to_string(), + }, + ], + }] + ); + + Ok(()) + } + #[tokio::test] async fn js_repl_does_not_expose_process_global() -> anyhow::Result<()> { if !can_run_js_repl_runtime_tests().await { diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index d95a2c49a7..4897b4ea6f 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -175,7 +175,6 @@ impl ToolRouter { call_id, tool_name, payload, - source, }; match self.registry.dispatch(invocation).await { diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 5d2c8e3d5c..d3b7195504 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -36,28 +36,37 @@ use image::GenericImageView; use image::ImageBuffer; use image::Rgba; use image::load_from_memory; +use pretty_assertions::assert_eq; use serde_json::Value; use tokio::time::Duration; use wiremock::BodyPrintLimit; use wiremock::MockServer; -fn find_image_message(body: &Value) -> Option<&Value> { +fn image_messages(body: &Value) -> Vec<&Value> { body.get("input") .and_then(Value::as_array) - .and_then(|items| { - items.iter().find(|item| { - item.get("type").and_then(Value::as_str) == Some("message") - && item - .get("content") - .and_then(Value::as_array) - .map(|content| { - content.iter().any(|span| { - span.get("type").and_then(Value::as_str) == Some("input_image") + .map(|items| { + items + .iter() + .filter(|item| { + item.get("type").and_then(Value::as_str) == Some("message") + && item + .get("content") + .and_then(Value::as_array) + .map(|content| { + content.iter().any(|span| { + span.get("type").and_then(Value::as_str) == Some("input_image") + }) }) - }) - .unwrap_or(false) - }) + .unwrap_or(false) + }) + .collect() }) + .unwrap_or_default() +} + +fn find_image_message(body: &Value) -> Option<&Value> { + image_messages(body).into_iter().next() } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -366,8 +375,16 @@ console.log(out.output?.body?.text ?? ""); ); let body = req.body_json(); - let image_message = - find_image_message(&body).expect("pending input image message not included in request"); + let image_messages = image_messages(&body); + assert_eq!( + image_messages.len(), + 1, + "js_repl view_image should inject exactly one pending input image message" + ); + let image_message = image_messages + .into_iter() + .next() + .expect("pending input image message not included in request"); let image_url = image_message .get("content") .and_then(Value::as_array) From e6bb5d85537ac3ce4264fa8b80e06bad8ab6a12e Mon Sep 17 00:00:00 2001 From: sayan-oai Date: Tue, 24 Feb 2026 19:33:32 -0800 Subject: [PATCH 03/10] chore: change catalog mode to enum (#12656) make presence of custom catalog more clear by changing to enum instead of bool. --- codex-rs/core/src/models_manager/manager.rs | 23 ++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 4b1da3e58a..fac1991b10 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -41,11 +41,20 @@ pub enum RefreshStrategy { OnlineIfUncached, } +/// How the manager's base catalog is sourced for the lifetime of the process. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CatalogMode { + /// Start from bundled `models.json` and allow cache/network refresh updates. + Default, + /// Use a caller-provided catalog as authoritative and do not mutate it via refresh. + Custom, +} + /// Coordinates remote model discovery plus cached metadata on disk. #[derive(Debug)] pub struct ModelsManager { remote_models: RwLock>, - has_custom_model_catalog: bool, + catalog_mode: CatalogMode, auth_manager: Arc, etag: RwLock>, cache_manager: ModelsCacheManager, @@ -65,7 +74,11 @@ impl ModelsManager { ) -> Self { let cache_path = codex_home.join(MODEL_CACHE_FILE); let cache_manager = ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL); - let has_custom_model_catalog = model_catalog.is_some(); + let catalog_mode = if model_catalog.is_some() { + CatalogMode::Custom + } else { + CatalogMode::Default + }; let remote_models = model_catalog .map(|catalog| catalog.models) .unwrap_or_else(|| { @@ -74,7 +87,7 @@ impl ModelsManager { }); Self { remote_models: RwLock::new(remote_models), - has_custom_model_catalog, + catalog_mode, auth_manager, etag: RwLock::new(None), cache_manager, @@ -217,7 +230,7 @@ impl ModelsManager { /// Refresh available models according to the specified strategy. async fn refresh_available_models(&self, refresh_strategy: RefreshStrategy) -> CoreResult<()> { // don't override the custom model catalog if one was provided by the user - if self.has_custom_model_catalog { + if matches!(self.catalog_mode, CatalogMode::Custom) { return Ok(()); } @@ -364,7 +377,7 @@ impl ModelsManager { Self::load_remote_models_from_file() .unwrap_or_else(|err| panic!("failed to load bundled models.json: {err}")), ), - has_custom_model_catalog: false, + catalog_mode: CatalogMode::Default, auth_manager, etag: RwLock::new(None), cache_manager, From 16ca527c80ea3d863be5dcac64ba37b1f8c97e47 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 24 Feb 2026 19:35:28 -0800 Subject: [PATCH 04/10] chore: migrate additional permissions to PermissionProfile (#12731) This PR replaces the old `additional_permissions.fs_read/fs_write` shape with a shared `PermissionProfile` model and wires it through the command approval, sandboxing, protocol, and TUI layers. The schema is adopted from the `SkillManifestPermissions`, which is also refactored to use this unified struct. This helps us easily expose permission profiles in app server/core as a follow-up. --- .../schema/json/EventMsg.json | 135 ++++++++++++-- .../codex_app_server_protocol.schemas.json | 133 ++++++++++++-- .../typescript/ExecApprovalRequestEvent.ts | 4 +- .../typescript/FileSystemPermissions.ts | 5 + ...Permissions.ts => MacOsAutomationValue.ts} | 2 +- .../schema/typescript/MacOsPermissions.ts | 7 + .../typescript/MacOsPreferencesValue.ts | 5 + .../schema/typescript/PermissionProfile.ts | 7 + .../schema/typescript/index.ts | 6 +- codex-rs/core/src/codex.rs | 4 +- codex-rs/core/src/sandboxing/mod.rs | 49 +++-- codex-rs/core/src/skills/loader.rs | 4 +- codex-rs/core/src/skills/permissions.rs | 169 ++++++++---------- codex-rs/core/src/tools/handlers/mod.rs | 10 +- codex-rs/core/src/tools/handlers/shell.rs | 4 +- .../core/src/tools/handlers/unified_exec.rs | 4 +- codex-rs/core/src/tools/runtimes/mod.rs | 4 +- codex-rs/core/src/tools/runtimes/shell.rs | 6 +- .../core/src/tools/runtimes/unified_exec.rs | 6 +- codex-rs/core/src/tools/spec.rs | 49 ++--- codex-rs/core/src/unified_exec/mod.rs | 4 +- .../core/tests/suite/request_permissions.rs | 68 ++++--- codex-rs/protocol/src/approvals.rs | 4 +- codex-rs/protocol/src/models.rs | 70 +++++++- .../on_request_rule_request_permission.md | 4 +- .../tui/src/bottom_pane/approval_overlay.rs | 72 ++++---- 26 files changed, 572 insertions(+), 263 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts rename codex-rs/app-server-protocol/schema/typescript/{AdditionalPermissions.ts => MacOsAutomationValue.ts} (62%) create mode 100644 codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index c81f69d109..5139e5da66 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -5,23 +5,6 @@ "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", "type": "string" }, - "AdditionalPermissions": { - "properties": { - "fs_read": { - "items": { - "type": "string" - }, - "type": "array" - }, - "fs_write": { - "items": { - "type": "string" - }, - "type": "array" - } - }, - "type": "object" - }, "AgentMessageContent": { "oneOf": [ { @@ -1633,7 +1616,7 @@ "additional_permissions": { "anyOf": [ { - "$ref": "#/definitions/AdditionalPermissions" + "$ref": "#/definitions/PermissionProfile" }, { "type": "null" @@ -3330,6 +3313,29 @@ } ] }, + "FileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, "FunctionCallOutputBody": { "anyOf": [ { @@ -3523,6 +3529,66 @@ ], "type": "string" }, + "MacOsAutomationValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "MacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "MacOsPreferencesValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "McpAuthStatus": { "enum": [ "unsupported", @@ -3840,6 +3906,37 @@ ], "type": "string" }, + "PermissionProfile": { + "properties": { + "file_system": { + "anyOf": [ + { + "$ref": "#/definitions/FileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "PlanItemArg": { "additionalProperties": false, "properties": { @@ -6945,7 +7042,7 @@ "additional_permissions": { "anyOf": [ { - "$ref": "#/definitions/AdditionalPermissions" + "$ref": "#/definitions/PermissionProfile" }, { "type": "null" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 859970ef66..11dcf41921 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -1,23 +1,6 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { - "AdditionalPermissions": { - "properties": { - "fs_read": { - "items": { - "type": "string" - }, - "type": "array" - }, - "fs_write": { - "items": { - "type": "string" - }, - "type": "array" - } - }, - "type": "object" - }, "AgentMessageContent": { "oneOf": [ { @@ -2694,7 +2677,7 @@ "additional_permissions": { "anyOf": [ { - "$ref": "#/definitions/AdditionalPermissions" + "$ref": "#/definitions/PermissionProfile" }, { "type": "null" @@ -4534,6 +4517,29 @@ "title": "FileChangeRequestApprovalResponse", "type": "object" }, + "FileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, "FuzzyFileSearchParams": { "$schema": "http://json-schema.org/draft-07/schema#", "properties": { @@ -4834,6 +4840,66 @@ "title": "JSONRPCResponse", "type": "object" }, + "MacOsAutomationValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "MacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "MacOsPreferencesValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "McpInvocation": { "properties": { "arguments": { @@ -5093,6 +5159,37 @@ } ] }, + "PermissionProfile": { + "properties": { + "file_system": { + "anyOf": [ + { + "$ref": "#/definitions/FileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "PlanItemArg": { "additionalProperties": false, "properties": { diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts index c777831e89..5144dab6e6 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts @@ -1,11 +1,11 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { AdditionalPermissions } from "./AdditionalPermissions"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; import type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; import type { ParsedCommand } from "./ParsedCommand"; +import type { PermissionProfile } from "./PermissionProfile"; export type ExecApprovalRequestEvent = { /** @@ -51,4 +51,4 @@ proposed_network_policy_amendments?: Array, /** * Optional additional filesystem permissions requested for this command. */ -additional_permissions?: AdditionalPermissions, parsed_cmd: Array, }; +additional_permissions?: PermissionProfile, parsed_cmd: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts new file mode 100644 index 0000000000..9e03768df3 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/FileSystemPermissions.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type FileSystemPermissions = { read: Array | null, write: Array | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/AdditionalPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/MacOsAutomationValue.ts similarity index 62% rename from codex-rs/app-server-protocol/schema/typescript/AdditionalPermissions.ts rename to codex-rs/app-server-protocol/schema/typescript/MacOsAutomationValue.ts index 363807a2aa..e351c319dd 100644 --- a/codex-rs/app-server-protocol/schema/typescript/AdditionalPermissions.ts +++ b/codex-rs/app-server-protocol/schema/typescript/MacOsAutomationValue.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type AdditionalPermissions = { fs_read?: Array, fs_write?: Array, }; +export type MacOsAutomationValue = boolean | Array; diff --git a/codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts new file mode 100644 index 0000000000..5c0792412c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/MacOsPermissions.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { MacOsAutomationValue } from "./MacOsAutomationValue"; +import type { MacOsPreferencesValue } from "./MacOsPreferencesValue"; + +export type MacOsPermissions = { preferences: MacOsPreferencesValue | null, automations: MacOsAutomationValue | null, accessibility: boolean | null, calendar: boolean | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts b/codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts new file mode 100644 index 0000000000..74a67ca1cb --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/MacOsPreferencesValue.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type MacOsPreferencesValue = boolean | string; diff --git a/codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts b/codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts new file mode 100644 index 0000000000..bfc0de27b7 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/PermissionProfile.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { FileSystemPermissions } from "./FileSystemPermissions"; +import type { MacOsPermissions } from "./MacOsPermissions"; + +export type PermissionProfile = { network: boolean | null, file_system: FileSystemPermissions | null, macos: MacOsPermissions | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 3a1281cf2a..cc323e963d 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -3,7 +3,6 @@ export type { AbsolutePathBuf } from "./AbsolutePathBuf"; export type { AddConversationListenerParams } from "./AddConversationListenerParams"; export type { AddConversationSubscriptionResponse } from "./AddConversationSubscriptionResponse"; -export type { AdditionalPermissions } from "./AdditionalPermissions"; export type { AgentMessageContent } from "./AgentMessageContent"; export type { AgentMessageContentDeltaEvent } from "./AgentMessageContentDeltaEvent"; export type { AgentMessageDeltaEvent } from "./AgentMessageDeltaEvent"; @@ -72,6 +71,7 @@ export type { ExecOutputStream } from "./ExecOutputStream"; export type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; export type { ExitedReviewModeEvent } from "./ExitedReviewModeEvent"; export type { FileChange } from "./FileChange"; +export type { FileSystemPermissions } from "./FileSystemPermissions"; export type { ForcedLoginMethod } from "./ForcedLoginMethod"; export type { ForkConversationParams } from "./ForkConversationParams"; export type { ForkConversationResponse } from "./ForkConversationResponse"; @@ -117,6 +117,9 @@ export type { LoginApiKeyResponse } from "./LoginApiKeyResponse"; export type { LoginChatGptCompleteNotification } from "./LoginChatGptCompleteNotification"; export type { LoginChatGptResponse } from "./LoginChatGptResponse"; export type { LogoutChatGptResponse } from "./LogoutChatGptResponse"; +export type { MacOsAutomationValue } from "./MacOsAutomationValue"; +export type { MacOsPermissions } from "./MacOsPermissions"; +export type { MacOsPreferencesValue } from "./MacOsPreferencesValue"; export type { McpAuthStatus } from "./McpAuthStatus"; export type { McpInvocation } from "./McpInvocation"; export type { McpListToolsResponseEvent } from "./McpListToolsResponseEvent"; @@ -141,6 +144,7 @@ export type { ParsedCommand } from "./ParsedCommand"; export type { PatchApplyBeginEvent } from "./PatchApplyBeginEvent"; export type { PatchApplyEndEvent } from "./PatchApplyEndEvent"; export type { PatchApplyStatus } from "./PatchApplyStatus"; +export type { PermissionProfile } from "./PermissionProfile"; export type { Personality } from "./Personality"; export type { PlanDeltaEvent } from "./PlanDeltaEvent"; export type { PlanItem } from "./PlanItem"; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index d9f383ef63..0fe5f28523 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -70,8 +70,8 @@ use codex_protocol::items::PlanItem; use codex_protocol::items::TurnItem; use codex_protocol::items::UserMessageItem; use codex_protocol::mcp::CallToolResult; -use codex_protocol::models::AdditionalPermissions; use codex_protocol::models::BaseInstructions; +use codex_protocol::models::PermissionProfile; use codex_protocol::models::format_allow_prefixes; use codex_protocol::openai_models::ModelInfo; use codex_protocol::protocol::FileChange; @@ -2571,7 +2571,7 @@ impl Session { reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, - additional_permissions: Option, + additional_permissions: Option, ) -> ReviewDecision { // command-level approvals use `call_id`. // `approval_id` is only present for subcommand callbacks (execve intercept) diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 70ca5588e0..56c7bff6a6 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -24,7 +24,8 @@ use crate::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR; use crate::tools::sandboxing::SandboxablePreference; use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::WindowsSandboxLevel; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::PermissionProfile; pub use codex_protocol::models::SandboxPermissions; use codex_protocol::protocol::ReadOnlyAccess; use codex_utils_absolute_path::AbsolutePathBuf; @@ -41,7 +42,7 @@ pub struct CommandSpec { pub env: HashMap, pub expiration: ExecExpiration, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, } @@ -95,14 +96,24 @@ pub(crate) enum SandboxTransformError { } pub(crate) fn normalize_additional_permissions( - additional_permissions: AdditionalPermissions, + additional_permissions: PermissionProfile, command_cwd: &Path, -) -> Result { - let fs_read = - normalize_permission_paths(additional_permissions.fs_read, command_cwd, "fs_read")?; - let fs_write = - normalize_permission_paths(additional_permissions.fs_write, command_cwd, "fs_write")?; - Ok(AdditionalPermissions { fs_read, fs_write }) +) -> Result { + let Some(file_system) = additional_permissions.file_system else { + return Ok(PermissionProfile::default()); + }; + let read = file_system + .read + .map(|paths| normalize_permission_paths(paths, command_cwd, "file_system.read")) + .transpose()?; + let write = file_system + .write + .map(|paths| normalize_permission_paths(paths, command_cwd, "file_system.write")) + .transpose()?; + Ok(PermissionProfile { + file_system: Some(FileSystemPermissions { read, write }), + ..Default::default() + }) } fn normalize_permission_paths( @@ -162,7 +173,7 @@ fn dedup_absolute_paths(paths: Vec) -> Vec { } fn additional_permission_roots( - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Result<(Vec, Vec), SandboxTransformError> { let to_abs = |paths: &[PathBuf]| { let mut out = Vec::with_capacity(paths.len()); @@ -179,8 +190,20 @@ fn additional_permission_roots( }; Ok(( - to_abs(&additional_permissions.fs_read)?, - to_abs(&additional_permissions.fs_write)?, + to_abs( + additional_permissions + .file_system + .as_ref() + .and_then(|file_system| file_system.read.as_deref()) + .unwrap_or_default(), + )?, + to_abs( + additional_permissions + .file_system + .as_ref() + .and_then(|file_system| file_system.write.as_deref()) + .unwrap_or_default(), + )?, )) } @@ -206,7 +229,7 @@ fn merge_read_only_access_with_additional_reads( fn sandbox_policy_with_additional_permissions( sandbox_policy: &SandboxPolicy, - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Result { if additional_permissions.is_empty() { return Ok(sandbox_policy.clone()); diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 960830649b..873f6cc097 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -12,10 +12,10 @@ use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; use crate::skills::model::SkillPolicy; use crate::skills::model::SkillToolDependency; -use crate::skills::permissions::SkillManifestPermissions; use crate::skills::permissions::compile_permission_profile; use crate::skills::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::SkillScope; use dirs::home_dir; use dunce::canonicalize as canonicalize_path; @@ -54,7 +54,7 @@ struct SkillMetadataFile { #[serde(default)] policy: Option, #[serde(default)] - permissions: Option, + permissions: Option, } #[derive(Debug, Default, Deserialize)] diff --git a/codex-rs/core/src/skills/permissions.rs b/codex-rs/core/src/skills/permissions.rs index 24f3c9444c..4019a31079 100644 --- a/codex-rs/core/src/skills/permissions.rs +++ b/codex-rs/core/src/skills/permissions.rs @@ -3,10 +3,15 @@ use std::path::Component; use std::path::Path; use std::path::PathBuf; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsAutomationValue; +use codex_protocol::models::MacOsPermissions; +#[cfg(target_os = "macos")] +use codex_protocol::models::MacOsPreferencesValue; +use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use dirs::home_dir; use dunce::canonicalize as canonicalize_path; -use serde::Deserialize; use tracing::warn; use crate::config::Constrained; @@ -20,63 +25,24 @@ use crate::seatbelt_permissions::MacOsSeatbeltProfileExtensions; #[cfg(not(target_os = "macos"))] type MacOsSeatbeltProfileExtensions = (); -#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] -pub(crate) struct SkillManifestPermissions { - #[serde(default)] - pub(crate) network: bool, - #[serde(default)] - pub(crate) file_system: SkillManifestFileSystemPermissions, - #[serde(default)] - pub(crate) macos: SkillManifestMacOsPermissions, -} - -#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] -pub(crate) struct SkillManifestFileSystemPermissions { - #[serde(default)] - pub(crate) read: Vec, - #[serde(default)] - pub(crate) write: Vec, -} - -#[derive(Debug, Clone, PartialEq, Eq, Default, Deserialize)] -pub(crate) struct SkillManifestMacOsPermissions { - #[serde(default)] - pub(crate) preferences: Option, - #[serde(default)] - pub(crate) automations: Option, - #[serde(default)] - pub(crate) accessibility: bool, - #[serde(default)] - pub(crate) calendar: bool, -} - -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -#[serde(untagged)] -pub(crate) enum MacOsPreferencesValue { - Bool(bool), - Mode(String), -} - -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -#[serde(untagged)] -pub(crate) enum MacOsAutomationValue { - Bool(bool), - BundleIds(Vec), -} - pub(crate) fn compile_permission_profile( skill_dir: &Path, - permissions: Option, + permissions: Option, ) -> Option { - let permissions = permissions?; + let PermissionProfile { + network, + file_system, + macos, + } = permissions?; + let file_system = file_system.unwrap_or_default(); let fs_read = normalize_permission_paths( skill_dir, - &permissions.file_system.read, + file_system.read.as_deref().unwrap_or_default(), "permissions.file_system.read", ); let fs_write = normalize_permission_paths( skill_dir, - &permissions.file_system.write, + file_system.write.as_deref().unwrap_or_default(), "permissions.file_system.write", ); let sandbox_policy = if !fs_write.is_empty() { @@ -90,7 +56,7 @@ pub(crate) fn compile_permission_profile( readable_roots: fs_read, } }, - network_access: permissions.network, + network_access: network.unwrap_or_default(), exclude_tmpdir_env_var: false, exclude_slash_tmp: false, } @@ -105,8 +71,9 @@ pub(crate) fn compile_permission_profile( // Default sandbox policy SandboxPolicy::new_read_only_policy() }; + let macos_permissions = macos.unwrap_or_default(); let macos_seatbelt_profile_extensions = - build_macos_seatbelt_profile_extensions(&permissions.macos); + build_macos_seatbelt_profile_extensions(&macos_permissions); Some(Permissions { approval_policy: Constrained::allow_any(AskForApproval::Never), @@ -121,7 +88,7 @@ pub(crate) fn compile_permission_profile( fn normalize_permission_paths( skill_dir: &Path, - values: &[String], + values: &[PathBuf], field: &str, ) -> Vec { let mut paths = Vec::new(); @@ -141,9 +108,10 @@ fn normalize_permission_paths( fn normalize_permission_path( skill_dir: &Path, - value: &str, + value: &Path, field: &str, ) -> Option { + let value = value.to_string_lossy(); let trimmed = value.trim(); if trimmed.is_empty() { warn!("ignoring {field}: value is empty"); @@ -151,11 +119,10 @@ fn normalize_permission_path( } let expanded = expand_home(trimmed); - let path = PathBuf::from(expanded); - let absolute = if path.is_absolute() { - path + let absolute = if expanded.is_absolute() { + expanded } else { - skill_dir.join(path) + skill_dir.join(expanded) }; let normalized = normalize_lexically(&absolute); let canonicalized = canonicalize_path(&normalized).unwrap_or(normalized); @@ -168,24 +135,24 @@ fn normalize_permission_path( } } -fn expand_home(path: &str) -> String { +fn expand_home(path: &str) -> PathBuf { if path == "~" { if let Some(home) = home_dir() { - return home.to_string_lossy().to_string(); + return home; } - return path.to_string(); + return PathBuf::from(path); } if let Some(rest) = path.strip_prefix("~/") && let Some(home) = home_dir() { - return home.join(rest).to_string_lossy().to_string(); + return home.join(rest); } - path.to_string() + PathBuf::from(path) } #[cfg(target_os = "macos")] fn build_macos_seatbelt_profile_extensions( - permissions: &SkillManifestMacOsPermissions, + permissions: &MacOsPermissions, ) -> Option { let defaults = MacOsSeatbeltProfileExtensions::default(); @@ -198,8 +165,10 @@ fn build_macos_seatbelt_profile_extensions( permissions.automations.as_ref(), defaults.macos_automation, ), - macos_accessibility: permissions.accessibility, - macos_calendar: permissions.calendar, + macos_accessibility: permissions + .accessibility + .unwrap_or(defaults.macos_accessibility), + macos_calendar: permissions.calendar.unwrap_or(defaults.macos_calendar), }; Some(extensions) } @@ -262,7 +231,7 @@ fn resolve_macos_automation_permission( #[cfg(not(target_os = "macos"))] fn build_macos_seatbelt_profile_extensions( - _: &SkillManifestMacOsPermissions, + _: &MacOsPermissions, ) -> Option { None } @@ -285,10 +254,6 @@ fn normalize_lexically(path: &Path) -> PathBuf { #[cfg(test)] mod tests { - use super::SkillManifestFileSystemPermissions; - #[cfg(target_os = "macos")] - use super::SkillManifestMacOsPermissions; - use super::SkillManifestPermissions; use super::compile_permission_profile; use crate::config::Constrained; use crate::config::Permissions; @@ -296,9 +261,18 @@ mod tests { use crate::protocol::AskForApproval; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; + use codex_protocol::models::FileSystemPermissions; + #[cfg(target_os = "macos")] + use codex_protocol::models::MacOsAutomationValue; + #[cfg(target_os = "macos")] + use codex_protocol::models::MacOsPermissions; + #[cfg(target_os = "macos")] + use codex_protocol::models::MacOsPreferencesValue; + use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; + use std::path::PathBuf; #[test] fn compile_permission_profile_normalizes_paths() { @@ -310,16 +284,16 @@ mod tests { let profile = compile_permission_profile( &skill_dir, - Some(SkillManifestPermissions { - network: true, - file_system: SkillManifestFileSystemPermissions { - read: vec![ - "./data".to_string(), - "./data".to_string(), - "scripts/../data".to_string(), - ], - write: vec!["./output".to_string()], - }, + Some(PermissionProfile { + network: Some(true), + file_system: Some(FileSystemPermissions { + read: Some(vec![ + PathBuf::from("./data"), + PathBuf::from("./data"), + PathBuf::from("scripts/../data"), + ]), + write: Some(vec![PathBuf::from("./output")]), + }), ..Default::default() }), ) @@ -380,8 +354,8 @@ mod tests { let profile = compile_permission_profile( &skill_dir, - Some(SkillManifestPermissions { - network: true, + Some(PermissionProfile { + network: Some(true), ..Default::default() }), ) @@ -415,12 +389,12 @@ mod tests { let profile = compile_permission_profile( &skill_dir, - Some(SkillManifestPermissions { - network: true, - file_system: SkillManifestFileSystemPermissions { - read: vec!["./data".to_string()], - write: Vec::new(), - }, + Some(PermissionProfile { + network: Some(true), + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("./data")]), + write: Some(Vec::new()), + }), ..Default::default() }), ) @@ -464,15 +438,15 @@ mod tests { let profile = compile_permission_profile( &skill_dir, - Some(SkillManifestPermissions { - macos: SkillManifestMacOsPermissions { - preferences: Some(super::MacOsPreferencesValue::Mode("readwrite".to_string())), - automations: Some(super::MacOsAutomationValue::BundleIds(vec![ + Some(PermissionProfile { + macos: Some(MacOsPermissions { + preferences: Some(MacOsPreferencesValue::Mode("readwrite".to_string())), + automations: Some(MacOsAutomationValue::BundleIds(vec![ "com.apple.Notes".to_string(), ])), - accessibility: true, - calendar: true, - }, + accessibility: Some(true), + calendar: Some(true), + }), ..Default::default() }), ) @@ -502,9 +476,8 @@ mod tests { let skill_dir = tempdir.path().join("skill"); fs::create_dir_all(&skill_dir).expect("skill dir"); - let profile = - compile_permission_profile(&skill_dir, Some(SkillManifestPermissions::default())) - .expect("profile"); + let profile = compile_permission_profile(&skill_dir, Some(PermissionProfile::default())) + .expect("profile"); assert_eq!( profile.macos_seatbelt_profile_extensions, diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index bdc47a088d..23dc979f2c 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -24,7 +24,7 @@ use crate::function_tool::FunctionCallError; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::normalize_additional_permissions; pub use apply_patch::ApplyPatchHandler; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; pub use dynamic::DynamicToolHandler; pub use grep_files::GrepFilesHandler; @@ -62,9 +62,9 @@ pub(super) fn normalize_and_validate_additional_permissions( request_permission_enabled: bool, approval_policy: AskForApproval, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option, cwd: &Path, -) -> Result, String> { +) -> Result, String> { let uses_additional_permissions = matches!( sandbox_permissions, SandboxPermissions::WithAdditionalPermissions @@ -87,14 +87,14 @@ pub(super) fn normalize_and_validate_additional_permissions( } let Some(additional_permissions) = additional_permissions else { return Err( - "missing `additional_permissions`; provide `fs_read` and/or `fs_write` when using `with_additional_permissions`" + "missing `additional_permissions`; provide `file_system.read` and/or `file_system.write` when using `with_additional_permissions`" .to_string(), ); }; let normalized = normalize_additional_permissions(additional_permissions, cwd)?; if normalized.is_empty() { return Err( - "`additional_permissions` must include at least one path in `fs_read` or `fs_write`" + "`additional_permissions` must include at least one path in `file_system.read` or `file_system.write`" .to_string(), ); } diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 0a0bb53476..95dbe05e54 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -33,7 +33,7 @@ use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::runtimes::shell::ShellRuntimeBackend; use crate::tools::sandboxing::ToolCtx; use crate::tools::spec::ShellCommandBackendConfig; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; pub struct ShellHandler; @@ -50,7 +50,7 @@ pub struct ShellCommandHandler { struct RunExecLikeArgs { tool_name: String, exec_params: ExecParams, - additional_permissions: Option, + additional_permissions: Option, prefix_rule: Option>, session: Arc, turn: Arc, diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 9c5e3bc4b3..57a109dd48 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -21,8 +21,8 @@ use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::UnifiedExecResponse; use crate::unified_exec::WriteStdinRequest; use async_trait::async_trait; -use codex_protocol::models::AdditionalPermissions; use codex_protocol::models::FunctionCallOutputBody; +use codex_protocol::models::PermissionProfile; use serde::Deserialize; use std::path::PathBuf; use std::sync::Arc; @@ -47,7 +47,7 @@ pub(crate) struct ExecCommandArgs { #[serde(default)] sandbox_permissions: SandboxPermissions, #[serde(default)] - additional_permissions: Option, + additional_permissions: Option, #[serde(default)] justification: Option, #[serde(default)] diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 6dc20002df..937ab341d8 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -10,7 +10,7 @@ use crate::sandboxing::CommandSpec; use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::tools::sandboxing::ToolError; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use std::collections::HashMap; use std::path::Path; @@ -26,7 +26,7 @@ pub(crate) fn build_command_spec( env: &HashMap, expiration: ExecExpiration, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option, justification: Option, ) -> Result { let (program, args) = command diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 0a5353755c..80d3b22ebb 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -31,7 +31,7 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::sandbox_override_for_first_attempt; use crate::tools::sandboxing::with_cached_approval; use codex_network_proxy::NetworkProxy; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::collections::HashMap; @@ -46,7 +46,7 @@ pub struct ShellRequest { pub explicit_env_overrides: HashMap, pub network: Option, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, pub exec_approval_requirement: ExecApprovalRequirement, } @@ -89,7 +89,7 @@ pub(crate) struct ApprovalKey { command: Vec, cwd: PathBuf, sandbox_permissions: SandboxPermissions, - additional_permissions: Option, + additional_permissions: Option, } impl ShellRuntime { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 893dae27e5..4b0c53d123 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -32,7 +32,7 @@ use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcess; use crate::unified_exec::UnifiedExecProcessManager; use codex_network_proxy::NetworkProxy; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::collections::HashMap; @@ -47,7 +47,7 @@ pub struct UnifiedExecRequest { pub network: Option, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, pub exec_approval_requirement: ExecApprovalRequirement, } @@ -58,7 +58,7 @@ pub struct UnifiedExecApprovalKey { pub cwd: PathBuf, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, } pub struct UnifiedExecRuntime<'a> { diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 3c9ab4bf3a..0f50cf8735 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -256,29 +256,36 @@ fn create_approval_parameters(request_permission_enabled: bool) -> BTreeMap, pub tty: bool, pub sandbox_permissions: SandboxPermissions, - pub additional_permissions: Option, + pub additional_permissions: Option, pub justification: Option, pub prefix_rule: Option>, } diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index d6003e4b85..cda482079a 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -5,7 +5,8 @@ use codex_core::config::Constrained; use codex_core::features::Feature; use codex_core::sandboxing::SandboxPermissions; use codex_protocol::config_types::ReasoningSummary; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::FileSystemPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; @@ -79,7 +80,7 @@ fn parse_result(item: &Value) -> CommandResult { fn shell_event_with_request_permissions( call_id: &str, command: &str, - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Result { let args = json!({ "command": command, @@ -184,9 +185,12 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res let _ = fs::remove_file(&requested_write); let call_id = "request_permissions_skip_approval"; let command = "touch requested-but-unused.txt"; - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![requested_write.clone()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![requested_write.clone()]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, command, &requested_permissions)?; @@ -266,9 +270,12 @@ async fn read_only_with_additional_permissions_widens_to_unrequested_cwd_write() "printf {:?} > {:?} && cat {:?}", "cwd-widened", unrequested_write, unrequested_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![requested_write.clone()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![requested_write.clone()]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -354,9 +361,12 @@ async fn read_only_with_additional_permissions_widens_to_unrequested_tmp_write() "printf {:?} > {:?} && cat {:?}", "tmp-widened", tmp_write, tmp_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![requested_write.clone()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![requested_write.clone()]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -442,13 +452,19 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> "printf {:?} > {:?} && cat {:?}", "outside-cwd-ok", outside_write, outside_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().to_path_buf()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().to_path_buf()]), + }), + ..Default::default() }; - let normalized_requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().canonicalize()?], + let normalized_requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().canonicalize()?]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; @@ -530,13 +546,19 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul "printf {:?} > {:?} && cat {:?}", "should-not-write", outside_write, outside_write ); - let requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().to_path_buf()], + let requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().to_path_buf()]), + }), + ..Default::default() }; - let normalized_requested_permissions = AdditionalPermissions { - fs_read: vec![], - fs_write: vec![outside_dir.path().canonicalize()?], + let normalized_requested_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![]), + write: Some(vec![outside_dir.path().canonicalize()?]), + }), + ..Default::default() }; let event = shell_event_with_request_permissions(call_id, &command, &requested_permissions)?; diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index e0f0c06254..f742dc6328 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::path::PathBuf; use crate::mcp::RequestId; -use crate::models::AdditionalPermissions; +use crate::models::PermissionProfile; use crate::parse_command::ParsedCommand; use crate::protocol::FileChange; use schemars::JsonSchema; @@ -106,7 +106,7 @@ pub struct ExecApprovalRequestEvent { /// Optional additional filesystem permissions requested for this command. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - pub additional_permissions: Option, + pub additional_permissions: Option, pub parsed_cmd: Vec, } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index c9fea3a295..ff5030c75e 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -53,16 +53,68 @@ impl SandboxPermissions { } #[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] -pub struct AdditionalPermissions { - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub fs_read: Vec, - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub fs_write: Vec, +pub struct FileSystemPermissions { + pub read: Option>, + pub write: Option>, } -impl AdditionalPermissions { +impl FileSystemPermissions { pub fn is_empty(&self) -> bool { - self.fs_read.is_empty() && self.fs_write.is_empty() + self.read.is_none() && self.write.is_none() + } +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +pub struct MacOsPermissions { + pub preferences: Option, + pub automations: Option, + pub accessibility: Option, + pub calendar: Option, +} + +impl MacOsPermissions { + pub fn is_empty(&self) -> bool { + self.preferences.is_none() + && self.automations.is_none() + && self.accessibility.is_none() + && self.calendar.is_none() + } +} + +#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[serde(untagged)] +pub enum MacOsPreferencesValue { + Bool(bool), + Mode(String), +} + +#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +#[serde(untagged)] +pub enum MacOsAutomationValue { + Bool(bool), + BundleIds(Vec), +} + +#[derive(Debug, Clone, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS)] +pub struct PermissionProfile { + pub network: Option, + pub file_system: Option, + pub macos: Option, +} + +impl PermissionProfile { + pub fn is_empty(&self) -> bool { + self.network.is_none() + && self + .file_system + .as_ref() + .map(FileSystemPermissions::is_empty) + .unwrap_or(true) + && self + .macos + .as_ref() + .map(MacOsPermissions::is_empty) + .unwrap_or(true) } } @@ -800,7 +852,7 @@ pub struct ShellToolCallParams { pub prefix_rule: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - pub additional_permissions: Option, + pub additional_permissions: Option, #[serde(skip_serializing_if = "Option::is_none")] pub justification: Option, } @@ -826,7 +878,7 @@ pub struct ShellCommandToolCallParams { pub prefix_rule: Option>, #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] - pub additional_permissions: Option, + pub additional_permissions: Option, #[serde(skip_serializing_if = "Option::is_none")] pub justification: Option, } diff --git a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md index 7a9f71e96a..1c9a3853a6 100644 --- a/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md +++ b/codex-rs/protocol/src/prompts/permissions/approval_policy/on_request_rule_request_permission.md @@ -8,8 +8,8 @@ When you need extra filesystem access for one command, use: - `sandbox_permissions: "with_additional_permissions"` - `additional_permissions` with one or both fields: - - `fs_read`: list of paths that need read access - - `fs_write`: list of paths that need write access + - `file_system.read`: list of paths that need read access + - `file_system.write`: list of paths that need write access This keeps execution inside the current sandbox policy, while adding only the requested permissions for that command, unless an exec-policy allow rule applies and authorizes running the command outside the sandbox. diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 76b1c3d60d..0cfd48bec9 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -18,7 +18,7 @@ use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use codex_core::features::Features; use codex_protocol::mcp::RequestId; -use codex_protocol::models::AdditionalPermissions; +use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ElicitationAction; use codex_protocol::protocol::ExecPolicyAmendment; use codex_protocol::protocol::FileChange; @@ -46,7 +46,7 @@ pub(crate) enum ApprovalRequest { reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, - additional_permissions: Option, + additional_permissions: Option, }, ApplyPatch { id: String, @@ -450,7 +450,7 @@ enum ApprovalVariant { command: Vec, network_approval_context: Option, proposed_execpolicy_amendment: Option, - additional_permissions: Option, + additional_permissions: Option, }, ApplyPatch { id: String, @@ -486,7 +486,7 @@ impl ApprovalOption { fn exec_options( proposed_execpolicy_amendment: Option, network_approval_context: Option<&NetworkApprovalContext>, - additional_permissions: Option<&AdditionalPermissions>, + additional_permissions: Option<&PermissionProfile>, ) -> Vec { if network_approval_context.is_some() { return vec![ @@ -562,26 +562,26 @@ fn exec_options( } fn format_additional_permissions_rule( - additional_permissions: &AdditionalPermissions, + additional_permissions: &PermissionProfile, ) -> Option { let mut parts = Vec::new(); - if !additional_permissions.fs_read.is_empty() { - let reads = additional_permissions - .fs_read - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); - parts.push(format!("read {reads}")); - } - if !additional_permissions.fs_write.is_empty() { - let writes = additional_permissions - .fs_write - .iter() - .map(|path| format!("`{}`", path.display())) - .collect::>() - .join(", "); - parts.push(format!("write {writes}")); + if let Some(file_system) = additional_permissions.file_system.as_ref() { + if let Some(read) = file_system.read.as_ref() { + let reads = read + .iter() + .map(|path| format!("`{}`", path.display())) + .collect::>() + .join(", "); + parts.push(format!("read {reads}")); + } + if let Some(write) = file_system.write.as_ref() { + let writes = write + .iter() + .map(|path| format!("`{}`", path.display())) + .collect::>() + .join(", "); + parts.push(format!("write {writes}")); + } } if parts.is_empty() { @@ -641,6 +641,7 @@ fn elicitation_options() -> Vec { mod tests { use super::*; use crate::app_event::AppEvent; + use codex_protocol::models::FileSystemPermissions; use codex_protocol::protocol::NetworkApprovalProtocol; use insta::assert_snapshot; use pretty_assertions::assert_eq; @@ -800,9 +801,12 @@ mod tests { #[test] fn additional_permissions_exec_options_hide_execpolicy_amendment() { - let additional_permissions = AdditionalPermissions { - fs_read: vec![PathBuf::from("/tmp/readme.txt")], - fs_write: vec![PathBuf::from("/tmp/out.txt")], + let additional_permissions = PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/readme.txt")]), + write: Some(vec![PathBuf::from("/tmp/out.txt")]), + }), + ..Default::default() }; let options = exec_options(None, None, Some(&additional_permissions)); @@ -826,9 +830,12 @@ mod tests { reason: None, network_approval_context: None, proposed_execpolicy_amendment: None, - additional_permissions: Some(AdditionalPermissions { - fs_read: vec![PathBuf::from("/tmp/readme.txt")], - fs_write: vec![PathBuf::from("/tmp/out.txt")], + additional_permissions: Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/readme.txt")]), + write: Some(vec![PathBuf::from("/tmp/out.txt")]), + }), + ..Default::default() }), }; @@ -862,9 +869,12 @@ mod tests { reason: Some("need filesystem access".into()), network_approval_context: None, proposed_execpolicy_amendment: None, - additional_permissions: Some(AdditionalPermissions { - fs_read: vec![PathBuf::from("/tmp/readme.txt")], - fs_write: vec![PathBuf::from("/tmp/out.txt")], + additional_permissions: Some(PermissionProfile { + file_system: Some(FileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/readme.txt")]), + write: Some(vec![PathBuf::from("/tmp/out.txt")]), + }), + ..Default::default() }), }; From 8f3f2c3c025e06acf9fd5e149810c2d08e224d84 Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Tue, 24 Feb 2026 21:04:34 -0800 Subject: [PATCH 05/10] tests(js_repl): stabilize CI runtime test execution (#12407) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Stabilize `js_repl` runtime test setup in CI and move tool-facing `js_repl` behavior coverage into integration tests. This is a test/CI change only. No production `js_repl` behavior change is intended. ## Why - Bazel test sandboxes (especially on macOS) could resolve a different `node` than the one installed by `actions/setup-node`, which caused `js_repl` runtime/version failures. - `js_repl` runtime tests depend on platform-specific sandbox/test-harness behavior, so they need explicit gating in a base-stability commit. - Several tests in the `js_repl` unit test module were actually black-box/tool-level behavior tests and fit better in the integration suite. ## Changes - Add `actions/setup-node` to the Bazel and Rust `Tests` workflows, using the exact version pinned in the repo’s Node version file. - In Bazel (non-Windows), pass `CODEX_JS_REPL_NODE_PATH=$(which node)` into test env so `js_repl` uses the `actions/setup-node` runtime inside Bazel tests. - Add a new integration test suite for `js_repl` tool behavior and register it in the core integration test suite module. - Move black-box `js_repl` behavior tests into the integration suite (persistence/TLA, builtin tool invocation, recursive self-call rejection, `process` isolation, blocked builtin imports). - Keep white-box manager/kernel tests in the `js_repl` unit test module. - Gate `js_repl` runtime tests to run only on macOS and only when a usable Node runtime is available (skip on other platforms / missing Node in this commit). ## Impact - Reduces `js_repl` CI failures caused by Node resolution drift in Bazel. - Improves test organization by separating tool-facing behavior tests from white-box manager/kernel tests. - Keeps the base commit stable while expanding `js_repl` runtime coverage. #### [git stack](https://github.com/magus/git-stack-cli) - ✅ `1` https://github.com/openai/codex/pull/12372 - 👉 `2` https://github.com/openai/codex/pull/12407 - ⏳ `3` https://github.com/openai/codex/pull/12185 - ⏳ `4` https://github.com/openai/codex/pull/10673 --- .github/workflows/bazel.yml | 12 ++ .github/workflows/rust-ci.yml | 4 + codex-rs/core/src/tools/js_repl/mod.rs | 207 +------------------ codex-rs/core/tests/suite/js_repl.rs | 267 +++++++++++++++++++++++++ codex-rs/core/tests/suite/mod.rs | 1 + 5 files changed, 291 insertions(+), 200 deletions(-) create mode 100644 codex-rs/core/tests/suite/js_repl.rs diff --git a/.github/workflows/bazel.yml b/.github/workflows/bazel.yml index 1a0a69035f..539e9cb8e0 100644 --- a/.github/workflows/bazel.yml +++ b/.github/workflows/bazel.yml @@ -47,6 +47,11 @@ jobs: steps: - uses: actions/checkout@v6 + - name: Set up Node.js for js_repl tests + uses: actions/setup-node@v6 + with: + node-version-file: codex-rs/node-version.txt + # Some integration tests rely on DotSlash being installed. # See https://github.com/openai/codex/pull/7617. - name: Install DotSlash @@ -156,6 +161,13 @@ jobs: --build_metadata=VISIBILITY=PUBLIC ) + if [[ "${RUNNER_OS:-}" != "Windows" ]]; then + # Bazel test sandboxes on macOS may resolve an older Homebrew `node` + # before the `actions/setup-node` runtime on PATH. + node_bin="$(which node)" + bazel_args+=("--test_env=CODEX_JS_REPL_NODE_PATH=${node_bin}") + fi + if [[ -n "${BUILDBUDDY_API_KEY:-}" ]]; then echo "BuildBuddy API key is available; using remote Bazel configuration." # Work around Bazel 9 remote repo contents cache / overlay materialization failures diff --git a/.github/workflows/rust-ci.yml b/.github/workflows/rust-ci.yml index 8e8825e9f8..3122a965a2 100644 --- a/.github/workflows/rust-ci.yml +++ b/.github/workflows/rust-ci.yml @@ -499,6 +499,10 @@ jobs: steps: - uses: actions/checkout@v6 + - name: Set up Node.js for js_repl tests + uses: actions/setup-node@v6 + with: + node-version-file: codex-rs/node-version.txt - name: Install Linux build dependencies if: ${{ runner.os == 'Linux' }} shell: bash diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 18695ab889..3ae9cff62f 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1636,6 +1636,12 @@ mod tests { } async fn can_run_js_repl_runtime_tests() -> bool { + // These white-box runtime tests rely on the unit-test harness and are + // only required on macOS. Linux uses the codex-linux-sandbox arg0 + // dispatch path, which is exercised in integration tests instead. + if !cfg!(target_os = "macos") { + return false; + } if std::env::var_os("CODEX_SANDBOX").is_some() { return false; } @@ -1669,47 +1675,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn js_repl_persists_top_level_bindings_and_supports_tla() -> anyhow::Result<()> { - if !can_run_js_repl_runtime_tests().await { - return Ok(()); - } - - let (session, turn) = make_session_and_context().await; - let session = Arc::new(session); - let turn = Arc::new(turn); - let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); - let manager = turn.js_repl.manager().await?; - - let first = manager - .execute( - Arc::clone(&session), - Arc::clone(&turn), - Arc::clone(&tracker), - JsReplArgs { - code: "let x = await Promise.resolve(41); console.log(x);".to_string(), - timeout_ms: Some(10_000), - }, - ) - .await?; - assert!(first.output.contains("41")); - - let second = manager - .execute( - Arc::clone(&session), - Arc::clone(&turn), - Arc::clone(&tracker), - JsReplArgs { - code: "console.log(x + 1);".to_string(), - timeout_ms: Some(10_000), - }, - ) - .await?; - - assert!(second.output.contains("42")); - Ok(()) - } - #[tokio::test] async fn js_repl_timeout_does_not_deadlock() -> anyhow::Result<()> { if !can_run_js_repl_runtime_tests().await { @@ -1965,108 +1930,9 @@ mod tests { Ok(()) } - #[tokio::test] - async fn js_repl_can_call_tools() -> anyhow::Result<()> { - if !can_run_js_repl_runtime_tests().await { - return Ok(()); - } - - let (session, mut turn) = make_session_and_context().await; - turn.approval_policy - .set(AskForApproval::Never) - .expect("test setup should allow updating approval policy"); - turn.sandbox_policy - .set(SandboxPolicy::DangerFullAccess) - .expect("test setup should allow updating sandbox policy"); - - let session = Arc::new(session); - let turn = Arc::new(turn); - let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); - let manager = turn.js_repl.manager().await?; - - let shell = manager - .execute( - Arc::clone(&session), - Arc::clone(&turn), - Arc::clone(&tracker), - JsReplArgs { - code: "const shellOut = await codex.tool(\"shell_command\", { command: \"printf js_repl_shell_ok\" }); console.log(JSON.stringify(shellOut));".to_string(), - timeout_ms: Some(15_000), - }, - ) - .await?; - assert!(shell.output.contains("js_repl_shell_ok")); - - let tool = manager - .execute( - Arc::clone(&session), - Arc::clone(&turn), - Arc::clone(&tracker), - JsReplArgs { - code: "const toolOut = await codex.tool(\"list_mcp_resources\", {}); console.log(toolOut.type);".to_string(), - timeout_ms: Some(15_000), - }, - ) - .await?; - assert!(tool.output.contains("function_call_output")); - Ok(()) - } - - #[tokio::test] - async fn js_repl_tool_call_rejects_recursive_js_repl_invocation() -> anyhow::Result<()> { - if !can_run_js_repl_runtime_tests().await { - return Ok(()); - } - - let (session, mut turn) = make_session_and_context().await; - turn.approval_policy - .set(AskForApproval::Never) - .expect("test setup should allow updating approval policy"); - turn.sandbox_policy - .set(SandboxPolicy::DangerFullAccess) - .expect("test setup should allow updating sandbox policy"); - - let session = Arc::new(session); - let turn = Arc::new(turn); - let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); - let manager = turn.js_repl.manager().await?; - - let result = manager - .execute( - session, - turn, - tracker, - JsReplArgs { - code: r#" -try { - await codex.tool("js_repl", "console.log('recursive')"); - console.log("unexpected-success"); -} catch (err) { - console.log(String(err)); -} -"# - .to_string(), - timeout_ms: Some(15_000), - }, - ) - .await?; - - assert!( - result.output.contains("js_repl cannot invoke itself"), - "expected recursion guard message, got output: {}", - result.output - ); - assert!( - !result.output.contains("unexpected-success"), - "recursive js_repl tool call unexpectedly succeeded: {}", - result.output - ); - Ok(()) - } - #[tokio::test] async fn js_repl_waits_for_unawaited_tool_calls_before_completion() -> anyhow::Result<()> { - if !can_run_js_repl_runtime_tests().await || cfg!(windows) { + if !can_run_js_repl_runtime_tests().await { return Ok(()); } @@ -2277,65 +2143,6 @@ console.log(out.type); Ok(()) } - - #[tokio::test] - async fn js_repl_does_not_expose_process_global() -> anyhow::Result<()> { - if !can_run_js_repl_runtime_tests().await { - return Ok(()); - } - - let (session, turn) = make_session_and_context().await; - let session = Arc::new(session); - let turn = Arc::new(turn); - let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); - let manager = turn.js_repl.manager().await?; - - let result = manager - .execute( - session, - turn, - tracker, - JsReplArgs { - code: "console.log(typeof process);".to_string(), - timeout_ms: Some(10_000), - }, - ) - .await?; - assert!(result.output.contains("undefined")); - Ok(()) - } - - #[tokio::test] - async fn js_repl_blocks_sensitive_builtin_imports() -> anyhow::Result<()> { - if !can_run_js_repl_runtime_tests().await { - return Ok(()); - } - - let (session, turn) = make_session_and_context().await; - let session = Arc::new(session); - let turn = Arc::new(turn); - let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default())); - let manager = turn.js_repl.manager().await?; - - let err = manager - .execute( - session, - turn, - tracker, - JsReplArgs { - code: "await import(\"node:process\");".to_string(), - timeout_ms: Some(10_000), - }, - ) - .await - .expect_err("node:process import should be blocked"); - assert!( - err.to_string() - .contains("Importing module \"node:process\" is not allowed in js_repl") - ); - Ok(()) - } - #[tokio::test] async fn js_repl_prefers_env_node_module_dirs_over_config() -> anyhow::Result<()> { if !can_run_js_repl_runtime_tests().await { diff --git a/codex-rs/core/tests/suite/js_repl.rs b/codex-rs/core/tests/suite/js_repl.rs new file mode 100644 index 0000000000..364b077262 --- /dev/null +++ b/codex-rs/core/tests/suite/js_repl.rs @@ -0,0 +1,267 @@ +#![allow(clippy::expect_used, clippy::unwrap_used)] + +use anyhow::Result; +use codex_core::features::Feature; +use core_test_support::responses; +use core_test_support::responses::ResponseMock; +use core_test_support::responses::ResponsesRequest; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_custom_tool_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::sse; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use which::which; +use wiremock::MockServer; + +fn custom_tool_output_text_and_success( + req: &ResponsesRequest, + call_id: &str, +) -> (String, Option) { + let (output, success) = req + .custom_tool_call_output_content_and_success(call_id) + .expect("custom tool output should be present"); + (output.unwrap_or_default(), success) +} + +async fn run_js_repl_turn( + server: &MockServer, + prompt: &str, + calls: &[(&str, &str)], +) -> Result { + let mut builder = test_codex().with_config(|config| { + config.features.enable(Feature::JsRepl); + }); + let test = builder.build(server).await?; + + let mut first_events = vec![ev_response_created("resp-1")]; + for (call_id, js_input) in calls { + first_events.push(ev_custom_tool_call(call_id, "js_repl", js_input)); + } + first_events.push(ev_completed("resp-1")); + responses::mount_sse_once(server, sse(first_events)).await; + + let second_mock = responses::mount_sse_once( + server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + test.submit_turn(prompt).await?; + Ok(second_mock) +} + +fn can_run_js_repl_runtime_tests() -> bool { + // In the base CI-stabilization commit, skip these integration tests only + // when the js_repl Node runtime is not available. A later stacked commit + // removes this automatic skip and requires them everywhere. + if let Some(path) = std::env::var_os("CODEX_JS_REPL_NODE_PATH") + && std::path::Path::new(&path).exists() + { + return true; + } + which("node").is_ok() +} +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_persists_top_level_bindings_and_supports_tla() -> Result<()> { + skip_if_no_network!(Ok(())); + + if !can_run_js_repl_runtime_tests() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let mut builder = test_codex().with_config(|config| { + config.features.enable(Feature::JsRepl); + }); + let test = builder.build(&server).await?; + + responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_custom_tool_call( + "call-1", + "js_repl", + "let x = await Promise.resolve(41); console.log(x);", + ), + ev_completed("resp-1"), + ]), + ) + .await; + let second_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-2"), + ev_custom_tool_call("call-2", "js_repl", "console.log(x + 1);"), + ev_completed("resp-2"), + ]), + ) + .await; + let third_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-3"), + ]), + ) + .await; + + test.submit_turn("run js_repl twice").await?; + + let req2 = second_mock.single_request(); + let (first_output, first_success) = custom_tool_output_text_and_success(&req2, "call-1"); + assert_ne!( + first_success, + Some(false), + "first js_repl call failed unexpectedly: {first_output}" + ); + assert!(first_output.contains("41")); + + let req3 = third_mock.single_request(); + let (second_output, second_success) = custom_tool_output_text_and_success(&req3, "call-2"); + assert_ne!( + second_success, + Some(false), + "second js_repl call failed unexpectedly: {second_output}" + ); + assert!(second_output.contains("42")); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_can_invoke_builtin_tools() -> Result<()> { + skip_if_no_network!(Ok(())); + + if !can_run_js_repl_runtime_tests() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let mock = run_js_repl_turn( + &server, + "use js_repl to call a tool", + &[( + "call-1", + "const toolOut = await codex.tool(\"list_mcp_resources\", {}); console.log(toolOut.type);", + )], + ) + .await?; + + let req = mock.single_request(); + let (output, success) = custom_tool_output_text_and_success(&req, "call-1"); + assert_ne!( + success, + Some(false), + "js_repl call failed unexpectedly: {output}" + ); + assert!(output.contains("function_call_output")); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_tool_call_rejects_recursive_js_repl_invocation() -> Result<()> { + skip_if_no_network!(Ok(())); + + if !can_run_js_repl_runtime_tests() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let mock = run_js_repl_turn( + &server, + "use js_repl recursively", + &[( + "call-1", + r#" +try { + await codex.tool("js_repl", "console.log('recursive')"); + console.log("unexpected-success"); +} catch (err) { + console.log(String(err)); +} +"#, + )], + ) + .await?; + + let req = mock.single_request(); + let (output, success) = custom_tool_output_text_and_success(&req, "call-1"); + assert_ne!( + success, + Some(false), + "js_repl call failed unexpectedly: {output}" + ); + assert!( + output.contains("js_repl cannot invoke itself"), + "expected recursion guard message, got output: {output}" + ); + assert!( + !output.contains("unexpected-success"), + "recursive js_repl call unexpectedly succeeded: {output}" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_does_not_expose_process_global() -> Result<()> { + skip_if_no_network!(Ok(())); + + if !can_run_js_repl_runtime_tests() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let mock = run_js_repl_turn( + &server, + "check process visibility", + &[("call-1", "console.log(typeof process);")], + ) + .await?; + + let req = mock.single_request(); + let (output, success) = custom_tool_output_text_and_success(&req, "call-1"); + assert_ne!( + success, + Some(false), + "js_repl call failed unexpectedly: {output}" + ); + assert!(output.contains("undefined")); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn js_repl_blocks_sensitive_builtin_imports() -> Result<()> { + skip_if_no_network!(Ok(())); + + if !can_run_js_repl_runtime_tests() { + return Ok(()); + } + + let server = responses::start_mock_server().await; + let mock = run_js_repl_turn( + &server, + "import a blocked module", + &[("call-1", "await import(\"node:process\");")], + ) + .await?; + + let req = mock.single_request(); + let (output, success) = custom_tool_output_text_and_success(&req, "call-1"); + assert_ne!( + success, + Some(true), + "blocked import unexpectedly succeeded: {output}" + ); + assert!(output.contains("Importing module \"node:process\" is not allowed in js_repl")); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index f3b64e1e7d..1f20aeee30 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -78,6 +78,7 @@ mod grep_files; mod hierarchical_agents; mod image_rollout; mod items; +mod js_repl; mod json_result; mod list_dir; mod live_cli; From 1151972fb26ef3f8f0e96821b59860c5aae4c60d Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 24 Feb 2026 21:16:35 -0800 Subject: [PATCH 06/10] feat: add experimental additionalPermissions to v2 command execution approval requests (#12737) This adds additionalPermissions to the app-server v2 item/commandExecution/requestApproval payload as an experimental field. The field is now exposed on CommandExecutionRequestApprovalParams and is populated from the existing core approval event when a command requests additional sandbox permissions. This PR also contains changes to make server requests to support experiment API. A real app server test client test: sample payload with experimental flag off: ``` { < "id": 0, < "method": "item/commandExecution/requestApproval", < "params": { < "command": "/bin/zsh -lc 'mkdir -p ~/some/test && touch ~/some/test/file'", < "commandActions": [ < { < "command": "mkdir -p '~/some/test'", < "type": "unknown" < }, < { < "command": "touch '~/some/test/file'", < "type": "unknown" < } < ], < "cwd": "/Users/celia/code/codex/codex-rs", < "itemId": "call_QLp0LWkQ1XkU6VW9T2vUZFWB", < "proposedExecpolicyAmendment": [ < "mkdir", < "-p", < "~/some/test" < ], < "reason": "Do you want to allow creating ~/some/test/file outside the workspace?", < "threadId": "019c9309-e209-7d82-a01b-dcf9556a354d", < "turnId": "019c9309-e27a-7f33-834f-6011e795c2d6" < } < } ``` with experimental flag on: ``` < { < "id": 0, < "method": "item/commandExecution/requestApproval", < "params": { < "additionalPermissions": { < "fileSystem": null, < "macos": null, < "network": true < }, < "command": "/bin/zsh -lc 'install -D /dev/null ~/some/test/file'", < "commandActions": [ < { < "command": "install -D /dev/null '~/some/test/file'", < "type": "unknown" < } < ], < "cwd": "/Users/celia/code/codex/codex-rs", < "itemId": "call_K3U4b3dRbj3eMCqslmncbGsq", < "proposedExecpolicyAmendment": [ < "install", < "-D" < ], < "reason": "Do you want to allow creating the file at ~/some/test/file outside the workspace sandbox?", < "threadId": "019c9303-3a8e-76e1-81bf-d67ac446d892", < "turnId": "019c9303-3af1-7143-88a1-73132f771234" < } < } ``` --- ...CommandExecutionRequestApprovalParams.json | 114 ++++++++++++ .../schema/json/ServerRequest.json | 114 ++++++++++++ .../codex_app_server_protocol.schemas.json | 91 ++++++++++ .../v2/AdditionalFileSystemPermissions.ts | 5 + .../v2/AdditionalMacOsPermissions.ts | 7 + .../v2/AdditionalPermissionProfile.ts | 7 + .../CommandExecutionRequestApprovalParams.ts | 5 + .../schema/typescript/v2/index.ts | 3 + codex-rs/app-server-protocol/src/export.rs | 16 ++ .../src/protocol/common.rs | 29 +++ .../app-server-protocol/src/protocol/v2.rs | 78 +++++++- codex-rs/app-server-test-client/src/lib.rs | 49 ++++- codex-rs/app-server/README.md | 4 +- .../app-server/src/bespoke_event_handling.rs | 5 + codex-rs/app-server/src/lib.rs | 15 ++ codex-rs/app-server/src/message_processor.rs | 2 +- codex-rs/app-server/src/transport.rs | 167 ++++++++++++++++++ 17 files changed, 703 insertions(+), 8 deletions(-) create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AdditionalMacOsPermissions.ts create mode 100644 codex-rs/app-server-protocol/schema/typescript/v2/AdditionalPermissionProfile.ts diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index b7ad3fe315..c8329d1b88 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -1,6 +1,97 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AdditionalFileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, + "AdditionalMacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "AdditionalPermissionProfile": { + "properties": { + "fileSystem": { + "anyOf": [ + { + "$ref": "#/definitions/AdditionalFileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/AdditionalMacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "CommandAction": { "oneOf": [ { @@ -111,6 +202,29 @@ } ] }, + "MacOsAutomationValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "MacOsPreferencesValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "NetworkApprovalContext": { "properties": { "host": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index fb73dc0edd..f2f47eaa4f 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -1,6 +1,97 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AdditionalFileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, + "AdditionalMacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "AdditionalPermissionProfile": { + "properties": { + "fileSystem": { + "anyOf": [ + { + "$ref": "#/definitions/AdditionalFileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/AdditionalMacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "ApplyPatchApprovalParams": { "properties": { "callId": { @@ -430,6 +521,29 @@ ], "type": "object" }, + "MacOsAutomationValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "items": { + "type": "string" + }, + "type": "array" + } + ] + }, + "MacOsPreferencesValue": { + "anyOf": [ + { + "type": "boolean" + }, + { + "type": "string" + } + ] + }, "NetworkApprovalContext": { "properties": { "host": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 11dcf41921..5aad0b0988 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -1,6 +1,97 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "AdditionalFileSystemPermissions": { + "properties": { + "read": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + }, + "write": { + "items": { + "type": "string" + }, + "type": [ + "array", + "null" + ] + } + }, + "type": "object" + }, + "AdditionalMacOsPermissions": { + "properties": { + "accessibility": { + "type": [ + "boolean", + "null" + ] + }, + "automations": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsAutomationValue" + }, + { + "type": "null" + } + ] + }, + "calendar": { + "type": [ + "boolean", + "null" + ] + }, + "preferences": { + "anyOf": [ + { + "$ref": "#/definitions/MacOsPreferencesValue" + }, + { + "type": "null" + } + ] + } + }, + "type": "object" + }, + "AdditionalPermissionProfile": { + "properties": { + "fileSystem": { + "anyOf": [ + { + "$ref": "#/definitions/AdditionalFileSystemPermissions" + }, + { + "type": "null" + } + ] + }, + "macos": { + "anyOf": [ + { + "$ref": "#/definitions/AdditionalMacOsPermissions" + }, + { + "type": "null" + } + ] + }, + "network": { + "type": [ + "boolean", + "null" + ] + } + }, + "type": "object" + }, "AgentMessageContent": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts new file mode 100644 index 0000000000..fb45464f99 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalFileSystemPermissions.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type AdditionalFileSystemPermissions = { read: Array | null, write: Array | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalMacOsPermissions.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalMacOsPermissions.ts new file mode 100644 index 0000000000..eae1ad810c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalMacOsPermissions.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { MacOsAutomationValue } from "../MacOsAutomationValue"; +import type { MacOsPreferencesValue } from "../MacOsPreferencesValue"; + +export type AdditionalMacOsPermissions = { preferences: MacOsPreferencesValue | null, automations: MacOsAutomationValue | null, accessibility: boolean | null, calendar: boolean | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalPermissionProfile.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalPermissionProfile.ts new file mode 100644 index 0000000000..4ca4dca120 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AdditionalPermissionProfile.ts @@ -0,0 +1,7 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AdditionalFileSystemPermissions } from "./AdditionalFileSystemPermissions"; +import type { AdditionalMacOsPermissions } from "./AdditionalMacOsPermissions"; + +export type AdditionalPermissionProfile = { network: boolean | null, fileSystem: AdditionalFileSystemPermissions | null, macos: AdditionalMacOsPermissions | null, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts index 4021e50a6a..a7c143c6a5 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionRequestApprovalParams.ts @@ -1,6 +1,7 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { AdditionalPermissionProfile } from "./AdditionalPermissionProfile"; import type { CommandAction } from "./CommandAction"; import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; import type { NetworkApprovalContext } from "./NetworkApprovalContext"; @@ -36,6 +37,10 @@ cwd?: string | null, * Best-effort parsed command actions for friendly display. */ commandActions?: Array | null, +/** + * Optional additional permissions requested for this command. + */ +additionalPermissions?: AdditionalPermissionProfile | null, /** * Optional proposed execpolicy amendment to allow similar commands without prompting. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts index 12e6e9adb1..d719801993 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/index.ts @@ -4,6 +4,9 @@ export type { Account } from "./Account"; export type { AccountLoginCompletedNotification } from "./AccountLoginCompletedNotification"; export type { AccountRateLimitsUpdatedNotification } from "./AccountRateLimitsUpdatedNotification"; export type { AccountUpdatedNotification } from "./AccountUpdatedNotification"; +export type { AdditionalFileSystemPermissions } from "./AdditionalFileSystemPermissions"; +export type { AdditionalMacOsPermissions } from "./AdditionalMacOsPermissions"; +export type { AdditionalPermissionProfile } from "./AdditionalPermissionProfile"; export type { AgentMessageDeltaNotification } from "./AgentMessageDeltaNotification"; export type { AnalyticsConfig } from "./AnalyticsConfig"; export type { AppBranding } from "./AppBranding"; diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index b6c8da90ea..b48651c03c 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -1947,6 +1947,15 @@ mod tests { let thread_start_ts = fs::read_to_string(output_dir.join("v2").join("ThreadStartParams.ts"))?; assert_eq!(thread_start_ts.contains("mockExperimentalField"), true); + let command_execution_request_approval_ts = fs::read_to_string( + output_dir + .join("v2") + .join("CommandExecutionRequestApprovalParams.ts"), + )?; + assert_eq!( + command_execution_request_approval_ts.contains("additionalPermissions"), + true + ); Ok(()) } @@ -2083,6 +2092,12 @@ export type Config = { stableField: Keep, unstableField: string | null } & ({ [k let thread_start_json = fs::read_to_string(output_dir.join("v2").join("ThreadStartParams.json"))?; assert_eq!(thread_start_json.contains("mockExperimentalField"), false); + let command_execution_request_approval_json = + fs::read_to_string(output_dir.join("CommandExecutionRequestApprovalParams.json"))?; + assert_eq!( + command_execution_request_approval_json.contains("additionalPermissions"), + false + ); let client_request_json = fs::read_to_string(output_dir.join("ClientRequest.json"))?; assert_eq!( @@ -2093,6 +2108,7 @@ export type Config = { stableField: Keep, unstableField: string | null } & ({ [k let bundle_json = fs::read_to_string(output_dir.join("codex_app_server_protocol.schemas.json"))?; assert_eq!(bundle_json.contains("mockExperimentalField"), false); + assert_eq!(bundle_json.contains("additionalPermissions"), false); assert_eq!(bundle_json.contains("MockExperimentalMethodParams"), false); assert_eq!( bundle_json.contains("MockExperimentalMethodResponse"), diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index f32925746f..9e1e6f2306 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -1371,4 +1371,33 @@ mod tests { let reason = crate::experimental_api::ExperimentalApi::experimental_reason(&request); assert_eq!(reason, Some("mock/experimentalMethod")); } + + #[test] + fn command_execution_request_approval_additional_permissions_is_marked_experimental() { + let params = v2::CommandExecutionRequestApprovalParams { + thread_id: "thr_123".to_string(), + turn_id: "turn_123".to_string(), + item_id: "call_123".to_string(), + approval_id: None, + reason: None, + network_approval_context: None, + command: Some("cat file".to_string()), + cwd: None, + command_actions: None, + additional_permissions: Some(v2::AdditionalPermissionProfile { + network: None, + file_system: Some(v2::AdditionalFileSystemPermissions { + read: Some(vec![std::path::PathBuf::from("/tmp/allowed")]), + write: None, + }), + macos: None, + }), + proposed_execpolicy_amendment: None, + }; + let reason = crate::experimental_api::ExperimentalApi::experimental_reason(¶ms); + assert_eq!( + reason, + Some("item/commandExecution/requestApproval.additionalPermissions") + ); + } } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index b1f6ceb5c1..41e5450685 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -20,7 +20,12 @@ use codex_protocol::items::TurnItem as CoreTurnItem; use codex_protocol::mcp::Resource as McpResource; use codex_protocol::mcp::ResourceTemplate as McpResourceTemplate; use codex_protocol::mcp::Tool as McpTool; +use codex_protocol::models::FileSystemPermissions as CoreFileSystemPermissions; +use codex_protocol::models::MacOsAutomationValue as CoreMacOsAutomationValue; +use codex_protocol::models::MacOsPermissions as CoreMacOsPermissions; +use codex_protocol::models::MacOsPreferencesValue as CoreMacOsPreferencesValue; use codex_protocol::models::MessagePhase; +use codex_protocol::models::PermissionProfile as CorePermissionProfile; use codex_protocol::models::ResponseItem; use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::ReasoningEffort; @@ -713,6 +718,63 @@ impl From for NetworkApprovalContext { } } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct AdditionalFileSystemPermissions { + pub read: Option>, + pub write: Option>, +} + +impl From for AdditionalFileSystemPermissions { + fn from(value: CoreFileSystemPermissions) -> Self { + Self { + read: value.read, + write: value.write, + } + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct AdditionalMacOsPermissions { + pub preferences: Option, + pub automations: Option, + pub accessibility: Option, + pub calendar: Option, +} + +impl From for AdditionalMacOsPermissions { + fn from(value: CoreMacOsPermissions) -> Self { + Self { + preferences: value.preferences, + automations: value.automations, + accessibility: value.accessibility, + calendar: value.calendar, + } + } +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct AdditionalPermissionProfile { + pub network: Option, + pub file_system: Option, + pub macos: Option, +} + +impl From for AdditionalPermissionProfile { + fn from(value: CorePermissionProfile) -> Self { + Self { + network: value.network, + file_system: value.file_system.map(AdditionalFileSystemPermissions::from), + macos: value.macos.map(AdditionalMacOsPermissions::from), + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] @@ -3359,7 +3421,7 @@ pub struct ContextCompactedNotification { pub turn_id: String, } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct CommandExecutionRequestApprovalParams { @@ -3396,12 +3458,26 @@ pub struct CommandExecutionRequestApprovalParams { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub command_actions: Option>, + /// Optional additional permissions requested for this command. + #[experimental("item/commandExecution/requestApproval.additionalPermissions")] + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional = nullable)] + pub additional_permissions: Option, /// Optional proposed execpolicy amendment to allow similar commands without prompting. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional = nullable)] pub proposed_execpolicy_amendment: Option, } +impl CommandExecutionRequestApprovalParams { + pub fn strip_experimental_fields(&mut self) { + // TODO: Avoid hardcoding individual experimental fields here. + // We need a generic outbound compatibility design for stripping or + // otherwise handling experimental server->client payloads. + self.additional_permissions = None; + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index 1cba215e26..f5a7fa9c44 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -171,6 +171,9 @@ enum CliCommand { }, /// Send a user message through the app-server V2 thread/turn APIs. SendMessageV2 { + /// Opt into experimental app-server methods and fields. + #[arg(long)] + experimental_api: bool, /// User message to send to Codex. user_message: String, }, @@ -260,9 +263,18 @@ pub fn run() -> Result<()> { let endpoint = resolve_endpoint(codex_bin, url)?; send_message(&endpoint, &config_overrides, user_message) } - CliCommand::SendMessageV2 { user_message } => { + CliCommand::SendMessageV2 { + experimental_api, + user_message, + } => { let endpoint = resolve_endpoint(codex_bin, url)?; - send_message_v2_endpoint(&endpoint, &config_overrides, user_message, &dynamic_tools) + send_message_v2_endpoint( + &endpoint, + &config_overrides, + user_message, + experimental_api, + &dynamic_tools, + ) } CliCommand::ResumeMessageV2 { thread_id, @@ -508,19 +520,31 @@ pub fn send_message_v2( dynamic_tools: &Option>, ) -> Result<()> { let endpoint = Endpoint::SpawnCodex(codex_bin.to_path_buf()); - send_message_v2_endpoint(&endpoint, config_overrides, user_message, dynamic_tools) + send_message_v2_endpoint( + &endpoint, + config_overrides, + user_message, + true, + dynamic_tools, + ) } fn send_message_v2_endpoint( endpoint: &Endpoint, config_overrides: &[String], user_message: String, + experimental_api: bool, dynamic_tools: &Option>, ) -> Result<()> { + if dynamic_tools.is_some() && !experimental_api { + bail!("--dynamic-tools requires --experimental-api for send-message-v2"); + } + send_message_v2_with_policies( endpoint, config_overrides, user_message, + experimental_api, None, None, dynamic_tools, @@ -690,6 +714,7 @@ fn trigger_cmd_approval( endpoint, config_overrides, message, + true, Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, @@ -711,6 +736,7 @@ fn trigger_patch_approval( endpoint, config_overrides, message, + true, Some(AskForApproval::OnRequest), Some(SandboxPolicy::ReadOnly { access: ReadOnlyAccess::FullAccess, @@ -729,6 +755,7 @@ fn no_trigger_cmd_approval( endpoint, config_overrides, prompt.to_string(), + true, None, None, dynamic_tools, @@ -739,13 +766,14 @@ fn send_message_v2_with_policies( endpoint: &Endpoint, config_overrides: &[String], user_message: String, + experimental_api: bool, approval_policy: Option, sandbox_policy: Option, dynamic_tools: &Option>, ) -> Result<()> { let mut client = CodexClient::connect(endpoint, config_overrides)?; - let initialize = client.initialize()?; + let initialize = client.initialize_with_experimental_api(experimental_api)?; println!("< initialize response: {initialize:?}"); let thread_response = client.thread_start(ThreadStartParams { @@ -1032,6 +1060,13 @@ impl CodexClient { } fn initialize(&mut self) -> Result { + self.initialize_with_experimental_api(true) + } + + fn initialize_with_experimental_api( + &mut self, + experimental_api: bool, + ) -> Result { let request_id = self.request_id(); let request = ClientRequest::Initialize { request_id: request_id.clone(), @@ -1042,7 +1077,7 @@ impl CodexClient { version: env!("CARGO_PKG_VERSION").to_string(), }, capabilities: Some(InitializeCapabilities { - experimental_api: true, + experimental_api, opt_out_notification_methods: Some( NOTIFICATIONS_TO_OPT_OUT .iter() @@ -1501,6 +1536,7 @@ impl CodexClient { command, cwd, command_actions, + additional_permissions, proposed_execpolicy_amendment, } = params; @@ -1527,6 +1563,9 @@ impl CodexClient { { println!("< command actions: {command_actions:?}"); } + if let Some(additional_permissions) = additional_permissions.as_ref() { + println!("< additional permissions: {additional_permissions:?}"); + } if let Some(execpolicy_amendment) = proposed_execpolicy_amendment.as_ref() { println!("< proposed execpolicy amendment: {execpolicy_amendment:?}"); } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 8ee15688c6..72f6861773 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -667,7 +667,7 @@ Certain actions (shell commands or modifying files) may require explicit user ap Order of messages: 1. `item/started` — shows the pending `commandExecution` item with `command`, `cwd`, and other fields so you can render the proposed action. -2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. +2. `item/commandExecution/requestApproval` (request) — carries the same `itemId`, `threadId`, `turnId`, optionally `approvalId` (for subcommand callbacks), and `reason`. For normal command approvals, it also includes `command`, `cwd`, and `commandActions` for friendly display. When `initialize.params.capabilities.experimentalApi = true`, it may also include experimental `additionalPermissions` describing requested per-command sandbox access. For network-only approvals, those command fields may be omitted and `networkApprovalContext` is provided instead. 3. Client response — `{ "decision": "accept", "acceptSettings": { "forSession": false } }` or `{ "decision": "decline" }`. 4. `item/completed` — final `commandExecution` item with `status: "completed" | "failed" | "declined"` and execution output. Render this as the authoritative result. @@ -1072,6 +1072,8 @@ At runtime, clients must send `initialize` with `capabilities.experimentalApi = 3. In `app-server-protocol/src/protocol/common.rs`, keep the method stable and use `inspect_params: true` when only some fields are experimental (like `thread/start`). If the entire method is experimental, annotate the method variant with `#[experimental("method/name")]`. +For server-initiated request payloads, annotate the field the same way so schema generation treats it as experimental, and make sure app-server omits that field when the client did not opt into `experimentalApi`. + 4. Regenerate protocol fixtures: ```bash diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index 2a5557b5ba..d94e9a0334 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -11,6 +11,7 @@ use crate::thread_state::TurnSummary; use crate::thread_status::ThreadWatchActiveGuard; use crate::thread_status::ThreadWatchManager; use codex_app_server_protocol::AccountRateLimitsUpdatedNotification; +use codex_app_server_protocol::AdditionalPermissionProfile as V2AdditionalPermissionProfile; use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::ApplyPatchApprovalParams; use codex_app_server_protocol::ApplyPatchApprovalResponse; @@ -267,6 +268,7 @@ pub(crate) async fn apply_bespoke_event_handling( reason, network_approval_context, proposed_execpolicy_amendment, + additional_permissions, parsed_cmd, .. } = ev; @@ -329,6 +331,8 @@ pub(crate) async fn apply_bespoke_event_handling( }; let proposed_execpolicy_amendment_v2 = proposed_execpolicy_amendment.map(V2ExecPolicyAmendment::from); + let additional_permissions = + additional_permissions.map(V2AdditionalPermissionProfile::from); let params = CommandExecutionRequestApprovalParams { thread_id: conversation_id.to_string(), @@ -340,6 +344,7 @@ pub(crate) async fn apply_bespoke_event_handling( command, cwd, command_actions, + additional_permissions, proposed_execpolicy_amendment: proposed_execpolicy_amendment_v2, }; let rx = outgoing diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 8607797e1b..5716dbb2d4 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -94,6 +94,7 @@ enum OutboundControlEvent { writer: mpsc::Sender, disconnect_sender: Option, initialized: Arc, + experimental_api_enabled: Arc, opted_out_notification_methods: Arc>>, }, /// Remove state for a closed/disconnected connection. @@ -503,6 +504,7 @@ pub async fn run_main_with_transport( writer, disconnect_sender, initialized, + experimental_api_enabled, opted_out_notification_methods, } => { outbound_connections.insert( @@ -510,6 +512,7 @@ pub async fn run_main_with_transport( OutboundConnectionState::new( writer, initialized, + experimental_api_enabled, opted_out_notification_methods, disconnect_sender, ), @@ -609,6 +612,8 @@ pub async fn run_main_with_transport( disconnect_sender, } => { let outbound_initialized = Arc::new(AtomicBool::new(false)); + let outbound_experimental_api_enabled = + Arc::new(AtomicBool::new(false)); let outbound_opted_out_notification_methods = Arc::new(RwLock::new(HashSet::new())); if outbound_control_tx @@ -617,6 +622,9 @@ pub async fn run_main_with_transport( writer, disconnect_sender, initialized: Arc::clone(&outbound_initialized), + experimental_api_enabled: Arc::clone( + &outbound_experimental_api_enabled, + ), opted_out_notification_methods: Arc::clone( &outbound_opted_out_notification_methods, ), @@ -630,6 +638,7 @@ pub async fn run_main_with_transport( connection_id, ConnectionState::new( outbound_initialized, + outbound_experimental_api_enabled, outbound_opted_out_notification_methods, ), ); @@ -679,6 +688,12 @@ pub async fn run_main_with_transport( "failed to update outbound opted-out notifications" ); } + connection_state + .outbound_experimental_api_enabled + .store( + connection_state.session.experimental_api_enabled, + std::sync::atomic::Ordering::Release, + ); if !was_initialized && connection_state.session.initialized { processor.send_initialize_notifications().await; } diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 70e1371ce2..8d5412d4e9 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -133,7 +133,7 @@ pub(crate) struct MessageProcessor { #[derive(Clone, Debug, Default)] pub(crate) struct ConnectionSessionState { pub(crate) initialized: bool, - experimental_api_enabled: bool, + pub(crate) experimental_api_enabled: bool, pub(crate) opted_out_notification_methods: HashSet, } diff --git a/codex-rs/app-server/src/transport.rs b/codex-rs/app-server/src/transport.rs index 027522153e..70a251ad2a 100644 --- a/codex-rs/app-server/src/transport.rs +++ b/codex-rs/app-server/src/transport.rs @@ -6,6 +6,7 @@ use crate::outgoing_message::OutgoingError; use crate::outgoing_message::OutgoingMessage; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::JSONRPCMessage; +use codex_app_server_protocol::ServerRequest; use futures::SinkExt; use futures::StreamExt; use owo_colors::OwoColorize; @@ -144,6 +145,7 @@ pub(crate) enum TransportEvent { pub(crate) struct ConnectionState { pub(crate) outbound_initialized: Arc, + pub(crate) outbound_experimental_api_enabled: Arc, pub(crate) outbound_opted_out_notification_methods: Arc>>, pub(crate) session: ConnectionSessionState, } @@ -151,10 +153,12 @@ pub(crate) struct ConnectionState { impl ConnectionState { pub(crate) fn new( outbound_initialized: Arc, + outbound_experimental_api_enabled: Arc, outbound_opted_out_notification_methods: Arc>>, ) -> Self { Self { outbound_initialized, + outbound_experimental_api_enabled, outbound_opted_out_notification_methods, session: ConnectionSessionState::default(), } @@ -163,6 +167,7 @@ impl ConnectionState { pub(crate) struct OutboundConnectionState { pub(crate) initialized: Arc, + pub(crate) experimental_api_enabled: Arc, pub(crate) opted_out_notification_methods: Arc>>, pub(crate) writer: mpsc::Sender, disconnect_sender: Option, @@ -172,11 +177,13 @@ impl OutboundConnectionState { pub(crate) fn new( writer: mpsc::Sender, initialized: Arc, + experimental_api_enabled: Arc, opted_out_notification_methods: Arc>>, disconnect_sender: Option, ) -> Self { Self { initialized, + experimental_api_enabled, opted_out_notification_methods, writer, disconnect_sender, @@ -577,6 +584,7 @@ async fn send_message_to_connection( warn!("dropping message for disconnected connection: {connection_id:?}"); return false; }; + let message = filter_outgoing_message_for_connection(connection_state, message); if should_skip_notification_for_connection(connection_state, &message) { return false; } @@ -602,6 +610,30 @@ async fn send_message_to_connection( } } +fn filter_outgoing_message_for_connection( + connection_state: &OutboundConnectionState, + message: OutgoingMessage, +) -> OutgoingMessage { + let experimental_api_enabled = connection_state + .experimental_api_enabled + .load(Ordering::Acquire); + match message { + OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { + request_id, + mut params, + }) => { + if !experimental_api_enabled { + params.strip_experimental_fields(); + } + OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { + request_id, + params, + }) + } + _ => message, + } +} + pub(crate) async fn route_outgoing_envelope( connections: &mut HashMap, envelope: OutgoingEnvelope, @@ -641,6 +673,7 @@ mod tests { use crate::error_code::OVERLOADED_ERROR_CODE; use pretty_assertions::assert_eq; use serde_json::json; + use std::path::PathBuf; use tokio::time::Duration; use tokio::time::timeout; @@ -880,6 +913,7 @@ mod tests { OutboundConnectionState::new( writer_tx, initialized, + Arc::new(AtomicBool::new(true)), opted_out_notification_methods, None, ), @@ -905,6 +939,136 @@ mod tests { ); } + #[tokio::test] + async fn command_execution_request_approval_strips_experimental_fields_without_capability() { + let connection_id = ConnectionId(8); + let (writer_tx, mut writer_rx) = mpsc::channel(1); + + let mut connections = HashMap::new(); + connections.insert( + connection_id, + OutboundConnectionState::new( + writer_tx, + Arc::new(AtomicBool::new(true)), + Arc::new(AtomicBool::new(false)), + Arc::new(RwLock::new(HashSet::new())), + None, + ), + ); + + route_outgoing_envelope( + &mut connections, + OutgoingEnvelope::ToConnection { + connection_id, + message: OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { + request_id: codex_app_server_protocol::RequestId::Integer(1), + params: codex_app_server_protocol::CommandExecutionRequestApprovalParams { + thread_id: "thr_123".to_string(), + turn_id: "turn_123".to_string(), + item_id: "call_123".to_string(), + approval_id: None, + reason: Some("Need extra read access".to_string()), + network_approval_context: None, + command: Some("cat file".to_string()), + cwd: Some(PathBuf::from("/tmp")), + command_actions: None, + additional_permissions: Some( + codex_app_server_protocol::AdditionalPermissionProfile { + network: None, + file_system: Some( + codex_app_server_protocol::AdditionalFileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/allowed")]), + write: None, + }, + ), + macos: None, + }, + ), + proposed_execpolicy_amendment: None, + }, + }), + }, + ) + .await; + + let message = writer_rx + .recv() + .await + .expect("request should be delivered to the connection"); + let json = serde_json::to_value(message).expect("request should serialize"); + assert_eq!(json["params"].get("additionalPermissions"), None); + } + + #[tokio::test] + async fn command_execution_request_approval_keeps_experimental_fields_with_capability() { + let connection_id = ConnectionId(9); + let (writer_tx, mut writer_rx) = mpsc::channel(1); + + let mut connections = HashMap::new(); + connections.insert( + connection_id, + OutboundConnectionState::new( + writer_tx, + Arc::new(AtomicBool::new(true)), + Arc::new(AtomicBool::new(true)), + Arc::new(RwLock::new(HashSet::new())), + None, + ), + ); + + route_outgoing_envelope( + &mut connections, + OutgoingEnvelope::ToConnection { + connection_id, + message: OutgoingMessage::Request(ServerRequest::CommandExecutionRequestApproval { + request_id: codex_app_server_protocol::RequestId::Integer(1), + params: codex_app_server_protocol::CommandExecutionRequestApprovalParams { + thread_id: "thr_123".to_string(), + turn_id: "turn_123".to_string(), + item_id: "call_123".to_string(), + approval_id: None, + reason: Some("Need extra read access".to_string()), + network_approval_context: None, + command: Some("cat file".to_string()), + cwd: Some(PathBuf::from("/tmp")), + command_actions: None, + additional_permissions: Some( + codex_app_server_protocol::AdditionalPermissionProfile { + network: None, + file_system: Some( + codex_app_server_protocol::AdditionalFileSystemPermissions { + read: Some(vec![PathBuf::from("/tmp/allowed")]), + write: None, + }, + ), + macos: None, + }, + ), + proposed_execpolicy_amendment: None, + }, + }), + }, + ) + .await; + + let message = writer_rx + .recv() + .await + .expect("request should be delivered to the connection"); + let json = serde_json::to_value(message).expect("request should serialize"); + assert_eq!( + json["params"]["additionalPermissions"], + json!({ + "network": null, + "fileSystem": { + "read": ["/tmp/allowed"], + "write": null, + }, + "macos": null, + }) + ); + } + #[tokio::test] async fn broadcast_does_not_block_on_slow_connection() { let fast_connection_id = ConnectionId(1); @@ -921,6 +1085,7 @@ mod tests { OutboundConnectionState::new( fast_writer_tx, Arc::new(AtomicBool::new(true)), + Arc::new(AtomicBool::new(true)), Arc::new(RwLock::new(HashSet::new())), Some(fast_disconnect_token.clone()), ), @@ -930,6 +1095,7 @@ mod tests { OutboundConnectionState::new( slow_writer_tx.clone(), Arc::new(AtomicBool::new(true)), + Arc::new(AtomicBool::new(true)), Arc::new(RwLock::new(HashSet::new())), Some(slow_disconnect_token.clone()), ), @@ -1006,6 +1172,7 @@ mod tests { OutboundConnectionState::new( writer_tx, Arc::new(AtomicBool::new(true)), + Arc::new(AtomicBool::new(true)), Arc::new(RwLock::new(HashSet::new())), None, ), From 6cb2f02ef8b73aaaa8f01e6aad04f333ce6e2a9a Mon Sep 17 00:00:00 2001 From: Curtis 'Fjord' Hawthorne Date: Tue, 24 Feb 2026 22:19:46 -0800 Subject: [PATCH 07/10] feat: update Docker image digest to reflect #12205 (#12372) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a clone of #12371 for easier rebasing/testing. #### [git stack](https://github.com/magus/git-stack-cli) - ✅ `1` https://github.com/openai/codex/pull/12407 - 👉 `2` https://github.com/openai/codex/pull/12372 - ⏳ `3` https://github.com/openai/codex/pull/12185 - ⏳ `4` https://github.com/openai/codex/pull/10673 Co-authored-by: Michael Bolin --- rbe.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rbe.bzl b/rbe.bzl index c1074c2be5..5d13812771 100644 --- a/rbe.bzl +++ b/rbe.bzl @@ -3,11 +3,11 @@ def _rbe_platform_repo_impl(rctx): if arch in ["x86_64", "amd64"]: cpu = "x86_64" exec_arch = "amd64" - image_sha = "8c9ff94187ea7c08a31e9a81f5fe8046ea3972a6768983c955c4079fa30567fb" + image_sha = "0a8e56bfaa3b2e5279db0d3bb2d62b44ba5e5d63a37d97eb8790f49da570af70" elif arch in ["aarch64", "arm64"]: cpu = "aarch64" exec_arch = "arm64" - image_sha = "ad9506086215fccfc66ed8d2be87847324be56790ae6a1964c241c28b77ef141" + image_sha = "136487cc4b7cf6f1021816ca18ed00896daed98404ea91dc4d6dd9e9d1cf9564" else: fail("Unsupported host arch for rbe platform: {}".format(arch)) From ddfa032eb8506eeb21b4f00f12f0836f06e8a6b0 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Feb 2026 22:27:05 -0800 Subject: [PATCH 08/10] fix: chatwidget was not honoring approval_id for an ExecApprovalRequestEvent (#12746) ## Why `ExecApprovalRequestEvent` can carry a distinct `approval_id` for subcommand approvals, including the `execve`-intercepted zsh-fork path. The session registers the pending approval callback under `approval_id` when one is present, but `ChatWidget` was stashing `call_id` in the approval modal state. When the user approved the command in the TUI, the response was sent back with the wrong identifier, so the pending approval could not be matched and the approval callback would not resolve. Note `approval_id` was introduced in https://github.com/openai/codex/pull/12051. ## What changed - In `tui/src/chatwidget.rs`, `ChatWidget` now uses `ExecApprovalRequestEvent::effective_approval_id()` when constructing `ApprovalRequest::Exec`. - That preserves the existing behavior for normal shell and `unified_exec` approvals, where `approval_id` is absent and the effective id still falls back to `call_id`. - For subcommand approvals that provide a distinct `approval_id`, the TUI now sends back the same key that `Session::request_command_approval()` registered. ## Verification - Traced the approval flow end to end to confirm the same effective approval id is now used on both sides of the round trip: - `Session::request_command_approval()` registers the pending callback under `approval_id.unwrap_or(call_id)`. - `ChatWidget` now emits `Op::ExecApproval` with that same effective id. --- .../tui/src/app/pending_interactive_replay.rs | 18 +++++---- codex-rs/tui/src/chatwidget.rs | 2 +- codex-rs/tui/src/chatwidget/tests.rs | 37 +++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index bc7fed6443..14d9f13c77 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -111,11 +111,12 @@ impl PendingInteractiveReplayState { pub(super) fn note_event(&mut self, event: &Event) { match &event.msg { EventMsg::ExecApprovalRequest(ev) => { - self.exec_approval_call_ids.insert(ev.call_id.clone()); + let approval_id = ev.effective_approval_id(); + self.exec_approval_call_ids.insert(approval_id.clone()); self.exec_approval_call_ids_by_turn_id .entry(ev.turn_id.clone()) .or_default() - .push(ev.call_id.clone()); + .push(approval_id); } EventMsg::ExecCommandBegin(ev) => { self.exec_approval_call_ids.remove(&ev.call_id); @@ -173,11 +174,12 @@ impl PendingInteractiveReplayState { pub(super) fn note_evicted_event(&mut self, event: &Event) { match &event.msg { EventMsg::ExecApprovalRequest(ev) => { - self.exec_approval_call_ids.remove(&ev.call_id); + let approval_id = ev.effective_approval_id(); + self.exec_approval_call_ids.remove(&approval_id); Self::remove_call_id_from_turn_map_entry( &mut self.exec_approval_call_ids_by_turn_id, &ev.turn_id, - &ev.call_id, + &approval_id, ); } EventMsg::ApplyPatchApprovalRequest(ev) => { @@ -218,7 +220,9 @@ impl PendingInteractiveReplayState { pub(super) fn should_replay_snapshot_event(&self, event: &Event) -> bool { match &event.msg { - EventMsg::ExecApprovalRequest(ev) => self.exec_approval_call_ids.contains(&ev.call_id), + EventMsg::ExecApprovalRequest(ev) => self + .exec_approval_call_ids + .contains(&ev.effective_approval_id()), EventMsg::ApplyPatchApprovalRequest(ev) => { self.patch_approval_call_ids.contains(&ev.call_id) } @@ -362,7 +366,7 @@ mod tests { } #[test] - fn thread_event_snapshot_drops_resolved_exec_approval_after_outbound_approval_call_id() { + fn thread_event_snapshot_drops_resolved_exec_approval_after_outbound_approval_id() { let mut store = ThreadEventStore::new(8); store.push_event(Event { id: "ev-1".to_string(), @@ -384,7 +388,7 @@ mod tests { }); store.note_outbound_op(&Op::ExecApproval { - id: "call-1".to_string(), + id: "approval-1".to_string(), turn_id: Some("turn-1".to_string()), decision: codex_protocol::protocol::ReviewDecision::Approved, }); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 15a6a58d62..9f4148b9d3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2569,7 +2569,7 @@ impl ChatWidget { self.notify(Notification::ExecApprovalRequested { command }); let request = ApprovalRequest::Exec { - id: ev.call_id, + id: ev.effective_approval_id(), command: ev.command, reason: ev.reason, network_approval_context: ev.network_approval_context, diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 1e938e2d4b..575b860bae 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2811,6 +2811,43 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { ); } +#[tokio::test] +async fn exec_approval_uses_approval_id_when_present() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_codex_event(Event { + id: "sub-short".into(), + msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { + call_id: "call-parent".into(), + approval_id: Some("approval-subcommand".into()), + turn_id: "turn-short".into(), + command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: Some( + "this is a test reason such as one that would be produced by the model".into(), + ), + network_approval_context: None, + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, + additional_permissions: None, + parsed_cmd: vec![], + }), + }); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE)); + + let mut found = false; + while let Ok(app_ev) = rx.try_recv() { + if let AppEvent::CodexOp(Op::ExecApproval { id, decision, .. }) = app_ev { + assert_eq!(id, "approval-subcommand"); + assert_matches!(decision, codex_protocol::protocol::ReviewDecision::Approved); + found = true; + break; + } + } + assert!(found, "expected ExecApproval op to be sent"); +} + #[tokio::test] async fn exec_approval_decision_truncates_multiline_and_long_commands() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; From 6bda5751326adfc99ca976fef74450896d2e4f43 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Feb 2026 22:28:56 -0800 Subject: [PATCH 09/10] feat: codex-shell-escalation should check skills for excalation policy --- .../tools/runtimes/shell/unix_escalation.rs | 167 +++++++++++---- codex-rs/core/tests/suite/skill_approval.rs | 197 +++++++++++++++++- 2 files changed, 319 insertions(+), 45 deletions(-) diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index b7d9b5a25f..b916140222 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -8,6 +8,7 @@ use crate::exec::is_likely_sandbox_denied; use crate::features::Feature; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; +use crate::skills::SkillMetadata; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; @@ -174,11 +175,12 @@ impl CoreShellActionProvider { async fn prompt( &self, - command: &[String], + program: &Path, + argv: &[String], workdir: &Path, stopwatch: &Stopwatch, ) -> anyhow::Result { - let command = command.to_vec(); + let command = join_program_and_argv(program, argv); let workdir = workdir.to_path_buf(); let session = self.session.clone(); let turn = self.turn.clone(); @@ -202,49 +204,41 @@ impl CoreShellActionProvider { }) .await) } -} -#[async_trait::async_trait] -impl EscalationPolicy for CoreShellActionProvider { - async fn determine_action( + /// Because we should be intercepting execve(2) calls, `program` should be + /// an absolute path. The idea is that we check to see whether it matches + /// any skills. + async fn find_skill(&self, program: &Path) -> Option { + let force_reload = false; + let skills_outcome = self + .session + .services + .skills_manager + .skills_for_cwd(&self.turn.cwd, force_reload) + .await; + + for skill in skills_outcome.skills { + // We intentionally ignore "enabled" status here for now. + let Some(skill_root) = skill.path_to_skills_md.parent() else { + continue; + }; + if program.starts_with(skill_root.join("scripts")) { + return Some(skill); + } + } + + None + } + + async fn process_decision( &self, - file: &Path, + decision: Decision, + needs_escalation: bool, + program: &Path, argv: &[String], workdir: &Path, ) -> anyhow::Result { - let command = std::iter::once(file.to_string_lossy().to_string()) - .chain(argv.iter().cloned()) - .collect::>(); - let (commands, used_complex_parsing) = - if let Some(commands) = parse_shell_lc_plain_commands(&command) { - (commands, false) - } else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) { - (vec![single_command], true) - } else { - (vec![command.clone()], false) - }; - - let fallback = |cmd: &[String]| { - crate::exec_policy::render_decision_for_unmatched_command( - self.approval_policy, - &self.sandbox_policy, - cmd, - self.sandbox_permissions, - used_complex_parsing, - ) - }; - let evaluation = { - let policy = self.policy.read().await; - policy.check_multiple(commands.iter(), &fallback) - }; - // When true, means the Evaluation was due to *.rules, not the - // fallback function. - let decision_driven_by_policy = - Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision); - let needs_escalation = - self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy; - - Ok(match evaluation.decision { + let action = match decision { Decision::Forbidden => EscalateAction::Deny { reason: Some("Execution forbidden by policy".to_string()), }, @@ -258,7 +252,7 @@ impl EscalationPolicy for CoreShellActionProvider { reason: Some("Execution forbidden by policy".to_string()), } } else { - match self.prompt(&command, workdir, &self.stopwatch).await? { + match self.prompt(program, argv, workdir, &self.stopwatch).await? { ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => { @@ -291,8 +285,88 @@ impl EscalationPolicy for CoreShellActionProvider { } } } - Decision::Allow => EscalateAction::Escalate, - }) + Decision::Allow => { + if needs_escalation { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + }; + tracing::info!( + "Policy decision for command {program:?} is {decision:?}, leading to escalation action {action:?}", + ); + Ok(action) + } +} + +#[async_trait::async_trait] +impl EscalationPolicy for CoreShellActionProvider { + async fn determine_action( + &self, + program: &Path, + argv: &[String], + workdir: &Path, + ) -> anyhow::Result { + tracing::info!( + "Determining escalation action for command {program:?} with args {argv:?} in {workdir:?}" + ); + + // In the usual case, the execve wrapper reports the command being + // executed in `program`, so a direct skill lookup is sufficient. + if let Some(skill) = self.find_skill(program).await { + // For now, we always prompt for scripts that look like they belong + // to skills, which means we ignore exec policy rules for those + // scripts. + tracing::info!("Matched {program:?} to skill {skill:?}, prompting for approval"); + // TODO(mbolin): We should read the permissions associated with the + // skill and use those specific permissions in the + // EscalateAction::Run case, rather than always escalating when a + // skill matches. + let needs_escalation = true; + return self + .process_decision(Decision::Prompt, needs_escalation, program, argv, workdir) + .await; + } + + let command = join_program_and_argv(program, argv); + let (commands, used_complex_parsing) = + if let Some(commands) = parse_shell_lc_plain_commands(&command) { + (commands, false) + } else if let Some(single_command) = parse_shell_lc_single_command_prefix(&command) { + (vec![single_command], true) + } else { + (vec![command.clone()], false) + }; + + let fallback = |cmd: &[String]| { + crate::exec_policy::render_decision_for_unmatched_command( + self.approval_policy, + &self.sandbox_policy, + cmd, + self.sandbox_permissions, + used_complex_parsing, + ) + }; + let evaluation = { + let policy = self.policy.read().await; + policy.check_multiple(commands.iter(), &fallback) + }; + // When true, means the Evaluation was due to *.rules, not the + // fallback function. + let decision_driven_by_policy = + Self::decision_driven_by_policy(&evaluation.matched_rules, evaluation.decision); + let needs_escalation = + self.sandbox_permissions.requires_escalated_permissions() || decision_driven_by_policy; + + self.process_decision( + evaluation.decision, + needs_escalation, + program, + argv, + workdir, + ) + .await } } @@ -403,6 +477,13 @@ fn map_exec_result( Ok(output) } +/// Some functions need the full command as a Vec. +fn join_program_and_argv(program: &Path, argv: &[String]) -> Vec { + std::iter::once(program.to_string_lossy().to_string()) + .chain(argv.iter().cloned()) + .collect::>() +} + #[cfg(test)] mod tests { use super::ParsedShellCommand; diff --git a/codex-rs/core/tests/suite/skill_approval.rs b/codex-rs/core/tests/suite/skill_approval.rs index de0b57886b..b6a2c816e6 100644 --- a/codex-rs/core/tests/suite/skill_approval.rs +++ b/codex-rs/core/tests/suite/skill_approval.rs @@ -2,8 +2,10 @@ use anyhow::Result; use codex_core::features::Feature; +use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; +use codex_protocol::protocol::SandboxPolicy; use codex_protocol::skill_approval::SkillApprovalResponse; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; @@ -59,6 +61,21 @@ fn command_for_script(script_path: &Path) -> Result { } async fn submit_turn(test: &TestCodex, prompt: &str) -> Result<()> { + submit_turn_with_policies( + test, + prompt, + AskForApproval::Never, + SandboxPolicy::DangerFullAccess, + ) + .await +} + +async fn submit_turn_with_policies( + test: &TestCodex, + prompt: &str, + approval_policy: AskForApproval, + sandbox_policy: SandboxPolicy, +) -> Result<()> { let session_model = test.session_configured.model.clone(); test.codex .submit(Op::UserTurn { @@ -68,8 +85,8 @@ async fn submit_turn(test: &TestCodex, prompt: &str) -> Result<()> { }], final_output_json_schema: None, cwd: test.cwd_path().to_path_buf(), - approval_policy: codex_protocol::protocol::AskForApproval::Never, - sandbox_policy: codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + approval_policy, + sandbox_policy, model: session_model, effort: None, summary: codex_protocol::config_types::ReasoningSummary::Auto, @@ -91,6 +108,65 @@ async fn wait_for_turn_complete_without_skill_approval(test: &TestCodex) { .await; } +#[cfg(unix)] +fn write_skill_with_shell_script(home: &Path, name: &str, script_name: &str) -> Result { + use std::os::unix::fs::PermissionsExt; + + let skill_dir = home.join("skills").join(name); + let scripts_dir = skill_dir.join("scripts"); + fs::create_dir_all(&scripts_dir)?; + fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {name} skill\n---\n"), + )?; + + let script_path = scripts_dir.join(script_name); + fs::write( + &script_path, + "#!/bin/sh\necho 'zsh-fork-stdout'\necho 'zsh-fork-stderr' >&2\n", + )?; + let mut permissions = fs::metadata(&script_path)?.permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&script_path, permissions)?; + Ok(script_path) +} + +#[cfg(unix)] +fn find_test_zsh_path() -> Result> { + use core_test_support::fetch_dotslash_file; + + let repo_root = codex_utils_cargo_bin::repo_root()?; + let dotslash_zsh = repo_root.join("codex-rs/app-server/tests/suite/zsh"); + if !dotslash_zsh.is_file() { + eprintln!( + "skipping zsh-fork skill test: shared zsh DotSlash file not found at {}", + dotslash_zsh.display() + ); + return Ok(None); + } + + match fetch_dotslash_file(&dotslash_zsh, None) { + Ok(path) => Ok(Some(path)), + Err(error) => { + eprintln!("skipping zsh-fork skill test: failed to fetch zsh via dotslash: {error:#}"); + Ok(None) + } + } +} + +#[cfg(unix)] +fn supports_exec_wrapper_intercept(zsh_path: &Path) -> bool { + let status = std::process::Command::new(zsh_path) + .arg("-fc") + .arg("/usr/bin/true") + .env("EXEC_WRAPPER", "/usr/bin/false") + .status(); + match status { + Ok(status) => !status.success(), + Err(_) => false, + } +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn skill_approval_event_round_trip_for_shell_command_skill_script_exec() -> Result<()> { skip_if_no_network!(Ok(())); @@ -317,3 +393,120 @@ async fn skill_approval_cache_is_per_skill() -> Result<()> { Ok(()) } + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn shell_zsh_fork_prompts_for_skill_script_execution() -> Result<()> { + use codex_config::Constrained; + use codex_protocol::protocol::ReviewDecision; + + skip_if_no_network!(Ok(())); + + let Some(zsh_path) = find_test_zsh_path()? else { + return Ok(()); + }; + if !supports_exec_wrapper_intercept(&zsh_path) { + eprintln!( + "skipping zsh-fork skill test: zsh does not support EXEC_WRAPPER intercepts ({})", + zsh_path.display() + ); + return Ok(()); + } + let Ok(main_execve_wrapper_exe) = codex_utils_cargo_bin::cargo_bin("codex-execve-wrapper") + else { + eprintln!("skipping zsh-fork skill test: unable to resolve `codex-execve-wrapper` binary"); + return Ok(()); + }; + + let server = start_mock_server().await; + let tool_call_id = "zsh-fork-skill-call"; + let mut builder = test_codex() + .with_pre_build_hook(|home| { + write_skill_with_shell_script(home, "mbolin-test-skill", "hello-mbolin.sh").unwrap(); + }) + .with_config(move |config| { + config.features.enable(Feature::ShellTool); + config.features.enable(Feature::ShellZshFork); + config.zsh_path = Some(zsh_path.clone()); + config.main_execve_wrapper_exe = Some(main_execve_wrapper_exe); + config.permissions.allow_login_shell = false; + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.permissions.sandbox_policy = + Constrained::allow_any(SandboxPolicy::new_workspace_write_policy()); + }); + let test = builder.build(&server).await?; + + let script_path = fs::canonicalize( + test.codex_home_path() + .join("skills/mbolin-test-skill/scripts/hello-mbolin.sh"), + )?; + let script_path_str = script_path.to_string_lossy().into_owned(); + let command = shlex::try_join([script_path_str.as_str()])?; + let arguments = shell_command_arguments(&command)?; + let mocks = + mount_function_call_agent_response(&server, tool_call_id, &arguments, "shell_command") + .await; + + submit_turn_with_policies( + &test, + "use $mbolin-test-skill", + AskForApproval::OnRequest, + SandboxPolicy::new_workspace_write_policy(), + ) + .await?; + + let maybe_approval = wait_for_event_match(test.codex.as_ref(), |event| match event { + EventMsg::ExecApprovalRequest(request) => Some(Some(request.clone())), + EventMsg::TurnComplete(_) => Some(None), + _ => None, + }) + .await; + let approval = match maybe_approval { + Some(approval) => approval, + None => { + let call_output = mocks + .completion + .single_request() + .function_call_output(tool_call_id); + panic!( + "expected exec approval request before completion; function_call_output={call_output:?}" + ); + } + }; + assert_eq!(approval.call_id, tool_call_id); + assert_eq!(approval.command.first(), Some(&script_path_str)); + assert!( + approval.command.iter().all(|arg| arg == &script_path_str), + "expected approval command to contain only the script path: {:?}", + approval.command + ); + + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + + wait_for_event(test.codex.as_ref(), |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + + let call_output = mocks + .completion + .single_request() + .function_call_output(tool_call_id); + let output = call_output["output"].as_str().unwrap_or_default(); + assert!( + output.contains("zsh-fork-stdout"), + "expected stdout marker in function_call_output: {output:?}" + ); + assert!( + output.contains("zsh-fork-stderr"), + "expected stderr marker in function_call_output: {output:?}" + ); + + Ok(()) +} From 76694799645cc34325e0e7dac4a8763d5daacd3f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Feb 2026 22:28:56 -0800 Subject: [PATCH 10/10] fix: found subtle bug in path resolution of execve program arg in codex-shell-escalation --- .../src/unix/escalate_protocol.rs | 5 +- .../src/unix/escalate_server.rs | 56 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/codex-rs/shell-escalation/src/unix/escalate_protocol.rs b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs index 6be00fe4b2..6bff482263 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_protocol.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_protocol.rs @@ -17,7 +17,10 @@ pub const LEGACY_BASH_EXEC_WRAPPER_ENV_VAR: &str = "BASH_EXEC_WRAPPER"; /// The client sends this to the server to request an exec() call. #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)] pub struct EscalateRequest { - /// The absolute path to the executable to run, i.e. the first arg to exec. + /// The executable path from the intercepted exec call. + /// + /// This may be relative, in which case it should be resolved against + /// `workdir`. pub file: PathBuf, /// The argv, including the program name (argv[0]). pub argv: Vec, diff --git a/codex-rs/shell-escalation/src/unix/escalate_server.rs b/codex-rs/shell-escalation/src/unix/escalate_server.rs index 56db822ebb..0c707b1263 100644 --- a/codex-rs/shell-escalation/src/unix/escalate_server.rs +++ b/codex-rs/shell-escalation/src/unix/escalate_server.rs @@ -152,8 +152,8 @@ async fn handle_escalate_session_with_policy( workdir, env, } = socket.receive::().await?; - let file = PathBuf::from(&file).absolutize()?.into_owned(); let workdir = PathBuf::from(&workdir).absolutize()?.into_owned(); + let file = file.absolutize_from(&workdir)?.into_owned(); let action = policy .determine_action(file.as_path(), &argv, &workdir) .await @@ -257,6 +257,25 @@ mod tests { } } + struct AssertingEscalationPolicy { + expected_file: PathBuf, + expected_workdir: PathBuf, + } + + #[async_trait::async_trait] + impl EscalationPolicy for AssertingEscalationPolicy { + async fn determine_action( + &self, + file: &Path, + _argv: &[String], + workdir: &Path, + ) -> anyhow::Result { + assert_eq!(file, self.expected_file.as_path()); + assert_eq!(workdir, self.expected_workdir.as_path()); + Ok(EscalateAction::Run) + } + } + #[tokio::test] async fn handle_escalate_session_respects_run_in_sandbox_decision() -> anyhow::Result<()> { let (server, client) = AsyncSocket::pair()?; @@ -292,6 +311,41 @@ mod tests { server_task.await? } + #[tokio::test] + async fn handle_escalate_session_resolves_relative_file_against_request_workdir() + -> anyhow::Result<()> { + let (server, client) = AsyncSocket::pair()?; + let tmp = tempfile::TempDir::new()?; + let workdir = tmp.path().join("workspace"); + std::fs::create_dir(&workdir)?; + let expected_file = workdir.join("bin/tool"); + let server_task = tokio::spawn(handle_escalate_session_with_policy( + server, + Arc::new(AssertingEscalationPolicy { + expected_file, + expected_workdir: workdir.clone(), + }), + )); + + client + .send(EscalateRequest { + file: PathBuf::from("./bin/tool"), + argv: vec!["./bin/tool".to_string()], + workdir, + env: HashMap::new(), + }) + .await?; + + let response = client.receive::().await?; + assert_eq!( + EscalateResponse { + action: EscalateAction::Run, + }, + response + ); + server_task.await? + } + #[tokio::test] async fn handle_escalate_session_executes_escalated_command() -> anyhow::Result<()> { let (server, client) = AsyncSocket::pair()?;