diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index e6f91f1661..fa85905bd7 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -2808,58 +2808,89 @@ async fn plugin_measurement_batch_emits_directly_and_filters_invalid_rows() { #[tokio::test] async fn command_execution_approval_response_publishes_user_review_event() { - let mut reducer = AnalyticsReducer::default(); - let mut events = Vec::new(); + for (kind, approval_id, subject, trigger) in [ + (None, None, "command_execution", "initial"), + ( + None, + Some("execve-approval"), + "command_execution", + "execve_intercept", + ), + ( + Some("writeStdin"), + Some("stdin-approval"), + "write_stdin", + "initial", + ), + ] { + let mut reducer = AnalyticsReducer::default(); + let mut events = Vec::new(); - ingest_review_prerequisites(&mut reducer, &mut events).await; - reducer - .ingest( - AnalyticsFact::ServerRequest { - connection_id: 7, - request: Box::new(sample_command_approval_request( - /*request_id*/ 41, /*approval_id*/ None, - )), - }, - &mut events, - ) - .await; - assert!(events.is_empty()); + ingest_review_prerequisites(&mut reducer, &mut events).await; + let mut request = serde_json::to_value(sample_command_approval_request( + /*request_id*/ 41, + approval_id, + )) + .expect("serialize approval request"); + // Missing kind models requests from older app-servers. + if let Some(kind) = kind { + request["params"]["kind"] = json!(kind); + } else { + request["params"].as_object_mut().unwrap().remove("kind"); + } + reducer + .ingest( + AnalyticsFact::ServerRequest { + connection_id: 7, + request: Box::new( + serde_json::from_value(request).expect("deserialize approval request"), + ), + }, + &mut events, + ) + .await; + assert!(events.is_empty()); - reducer - .ingest( - AnalyticsFact::ServerResponse { - completed_at_ms: 1_042, - response: Box::new(sample_command_approval_response( - /*request_id*/ 41, - CommandExecutionApprovalDecision::Accept, - )), - }, - &mut events, - ) - .await; + reducer + .ingest( + AnalyticsFact::ServerResponse { + completed_at_ms: 1_042, + response: Box::new(sample_command_approval_response( + /*request_id*/ 41, + CommandExecutionApprovalDecision::Accept, + )), + }, + &mut events, + ) + .await; - let payload = serde_json::to_value(&events).expect("serialize events"); - assert_eq!(payload.as_array().expect("events array").len(), 1); - assert_eq!(payload[0]["event_type"], "codex_review_event"); - assert_eq!(payload[0]["event_params"]["thread_id"], "thread-1"); - assert_eq!(payload[0]["event_params"]["turn_id"], "turn-1"); - assert_eq!(payload[0]["event_params"]["item_id"], "item-1"); - assert_eq!(payload[0]["event_params"]["review_id"], "user:41"); - assert_eq!(payload[0]["event_params"]["thread_source"], "user"); - assert_eq!( - payload[0]["event_params"]["subject_kind"], - "command_execution" - ); - assert_eq!( - payload[0]["event_params"]["subject_name"], - "command_execution" - ); - assert_eq!(payload[0]["event_params"]["reviewer"], "user"); - assert_eq!(payload[0]["event_params"]["trigger"], "initial"); - assert_eq!(payload[0]["event_params"]["status"], "approved"); - assert_eq!(payload[0]["event_params"]["started_at_ms"], 1_000); - assert_eq!(payload[0]["event_params"]["completed_at_ms"], 1_042); - assert_eq!(payload[0]["event_params"]["duration_ms"], 42); + let payload = serde_json::to_value(&events).expect("serialize events"); + assert_eq!(payload.as_array().expect("events array").len(), 1); + assert_eq!(payload[0]["event_type"], "codex_review_event"); + assert_eq!(payload[0]["event_params"]["thread_id"], "thread-1"); + assert_eq!(payload[0]["event_params"]["turn_id"], "turn-1"); + assert_eq!(payload[0]["event_params"]["item_id"], "item-1"); + assert_eq!(payload[0]["event_params"]["review_id"], "user:41"); + assert_eq!(payload[0]["event_params"]["thread_source"], "user"); + assert_eq!(payload[0]["event_params"]["subject_kind"], subject); + assert_eq!(payload[0]["event_params"]["subject_name"], subject); + assert_eq!(payload[0]["event_params"]["reviewer"], "user"); + assert_eq!(payload[0]["event_params"]["trigger"], trigger); + assert_eq!(payload[0]["event_params"]["status"], "approved"); + assert_eq!(payload[0]["event_params"]["started_at_ms"], 1_000); + assert_eq!(payload[0]["event_params"]["completed_at_ms"], 1_042); + assert_eq!(payload[0]["event_params"]["duration_ms"], 42); + + // Stdin reviews must not count toward the parent command's approval summary. + events.clear(); + ingest_completed_command_execution_item(&mut reducer, &mut events, "thread-1", "item-1") + .await; + let item = serde_json::to_value(&events[0]).expect("serialize tool item event"); + assert_eq!( + item["event_params"]["review_count"], + u64::from(kind.is_none()) + ); + } } async fn ingest_code_mode_facts( diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index 2b4f7cd669..43c8918551 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -126,6 +126,7 @@ use codex_app_server_protocol::CollabAgentTool; use codex_app_server_protocol::CollabAgentToolCallStatus; use codex_app_server_protocol::CommandAction; use codex_app_server_protocol::CommandExecutionApprovalDecision; +use codex_app_server_protocol::CommandExecutionApprovalKind; use codex_app_server_protocol::CommandExecutionSource; use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::DynamicToolCallOutputContentItem; @@ -1498,6 +1499,10 @@ impl AnalyticsReducer { fn ingest_server_request(&mut self, _connection_id: u64, request: ServerRequest) { match request { ServerRequest::CommandExecutionRequestApproval { request_id, params } => { + let is_stdin_review = match params.kind { + CommandExecutionApprovalKind::WriteStdin => true, + CommandExecutionApprovalKind::Command => false, + }; let is_network_access_review = params.network_approval_context.is_some(); let requested_network_access = is_network_access_review || params @@ -1511,7 +1516,9 @@ impl AnalyticsReducer { .and_then(|network| network.enabled) .unwrap_or(false); let requested_additional_permissions = params.additional_permissions.is_some(); - let trigger = if params.approval_id.is_some() { + let trigger = if is_stdin_review { + ReviewTrigger::Initial + } else if params.approval_id.is_some() { ReviewTrigger::ExecveIntercept } else if requested_network_access { ReviewTrigger::NetworkPolicyDenial @@ -1530,12 +1537,16 @@ impl AnalyticsReducer { turn_id: params.turn_id, item_id: Some(params.item_id), review_id: user_review_id(&request_id), - subject_kind: if is_network_access_review { + subject_kind: if is_stdin_review { + ReviewSubjectKind::WriteStdin + } else if is_network_access_review { ReviewSubjectKind::NetworkAccess } else { ReviewSubjectKind::CommandExecution }, - subject_name: if is_network_access_review { + subject_name: if is_stdin_review { + "write_stdin".to_string() + } else if is_network_access_review { "network_access".to_string() } else { "command_execution".to_string() diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index 3680793d49..a78ce21eb9 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 6a4c3c8fb4..07c9df0acf 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1822,6 +1822,8 @@ Order of messages: `kind` distinguishes command approvals from writes to an existing terminal. Requests from older servers without `kind` retain `command` semantics; `approvalId` alone does not distinguish stdin writes from execve interception. +When stdin approvals are enabled, a `write_stdin` approval sets `kind: "writeStdin"`, references the original terminal command's `itemId`, and has its own `approvalId`. The request belongs to the current turn, which may differ from the turn that opened the terminal. With `approvalsReviewer: "auto_review"`, the `item/autoApprovalReview/*` notifications likewise target the original command item and carry an action of type `writeStdin` with `approvalId`, `processId`, `stdin`, and `cwd`. For stdin approvals, `cwd` is the terminal’s launch directory, not its current working directory. Approving or denying a stdin write does not start, complete, or change the status of the parent command-execution item. + ### File change approvals Order of messages: diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index f14d4792ed..b05d6f4d6e 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -117,6 +117,7 @@ use codex_sandboxing::policy_transforms::intersect_permission_profiles; use codex_shell_command::parse_command::shlex_join; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::LegacyAppPathString; +use codex_utils_path_uri::PathUri; use std::collections::HashMap; use std::sync::Arc; use std::time::SystemTime; @@ -630,26 +631,46 @@ pub(crate) async fn apply_bespoke_event_handling( parsed_cmd, .. } = ev; - let command_actions = parsed_cmd - .iter() - .cloned() - .map(|parsed| V2ParsedCommand::from_core_with_cwd(parsed, &cwd)) - .collect::>(); + let cwd_uri = match PathUri::try_from(cwd.clone()) { + Ok(cwd) => cwd, + Err(err) => { + error!(%err, "invalid command approval cwd"); + if let Err(err) = conversation + .submit(Op::ExecApproval { + id: approval_id.unwrap_or(call_id), + turn_id: Some(turn_id), + decision: ReviewDecision::denied("invalid command approval cwd"), + }) + .await + { + error!(%err, "failed to reject invalid command approval"); + } + return; + } + }; + let command_presentation = + CommandExecutionPresentation::from_raw(&command, &parsed_cmd, &cwd_uri); + // Approval requests retain the exact command; only history is redacted. + let command_actions = match cwd_uri.to_abs_path() { + Ok(native_cwd) => parsed_cmd + .iter() + .cloned() + .map(|parsed| V2ParsedCommand::from_core_with_cwd(parsed, &native_cwd)) + .collect(), + Err(_) => vec![V2ParsedCommand::Unknown { + command: shlex_join(&command), + }], + }; let presentation = if let Some(network_approval_context) = network_approval_context.map(V2NetworkApprovalContext::from) { CommandExecutionApprovalPresentation::Network(network_approval_context) } else { - let command_presentation = CommandExecutionPresentation::from_raw( - &command, - &parsed_cmd, - &cwd.clone().into(), - ); let completion_item = CommandExecutionCompletionItem { plugin_id, script_path, command: command_presentation.command, - cwd: cwd.clone().into(), + cwd: cwd.clone(), command_actions: command_presentation.command_actions, }; CommandExecutionApprovalPresentation::Command(completion_item) diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index eca98d5cb1..0af689ee60 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -993,6 +993,9 @@ }, "workspace_owner_usage_nudge": { "type": "boolean" + }, + "write_stdin_approval": { + "type": "boolean" } }, "type": "object" @@ -6143,6 +6146,9 @@ }, "workspace_owner_usage_nudge": { "type": "boolean" + }, + "write_stdin_approval": { + "type": "boolean" } }, "type": "object" diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index 37f67cb719..390bc25942 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -26,16 +26,10 @@ pub(crate) enum GuardianApprovalRequest { justification: Option, tty: bool, }, - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "Constructed by the follow-up stdin approval routing change" - ) - )] WriteStdin { id: String, approval_id: String, + environment_id: String, process_id: i32, input: String, cwd: PathUri, @@ -129,6 +123,7 @@ struct CommandApprovalAction<'a> { #[derive(Serialize)] struct WriteStdinApprovalAction<'a> { tool: &'static str, + environment_id: &'a str, session_id: i32, chars: &'a str, cwd: LegacyAppPathString, @@ -300,6 +295,7 @@ pub(crate) fn guardian_approval_request_to_json( Some(*tty), ), GuardianApprovalRequest::WriteStdin { + environment_id, process_id, input, cwd, @@ -307,6 +303,7 @@ pub(crate) fn guardian_approval_request_to_json( .. } => serialize_guardian_action(WriteStdinApprovalAction { tool: "write_stdin", + environment_id, session_id: *process_id, chars: input, cwd: cwd.clone().into(), diff --git a/codex-rs/core/src/guardian/prompt.rs b/codex-rs/core/src/guardian/prompt.rs index 5e97f258f1..125228c619 100644 --- a/codex-rs/core/src/guardian/prompt.rs +++ b/codex-rs/core/src/guardian/prompt.rs @@ -313,10 +313,12 @@ pub(crate) async fn build_guardian_prompt_items_with_parent_turn( push_text("Retry reason:\n".to_string()); push_text(format!("{reason}\n\n")); } - push_text( + let action_scope = if matches!(&request, GuardianApprovalRequest::WriteStdin { .. }) { + "Assess input to the existing terminal, not a fresh command. The `cwd` field is its launch directory; the terminal's current directory and state may have changed. Use the retained transcript and read-only checks when that state matters.\n" + } else { "Assess the exact planned action below. Use read-only tool checks when local state matters.\n" - .to_string(), - ); + }; + push_text(action_scope.to_string()); push_text("Planned action JSON:\n".to_string()); } } diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index 34e019fcc6..f5694761c3 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -1371,6 +1371,7 @@ fn guardian_write_stdin_preserves_input_and_foreign_cwd() -> serde_json::Result< let action = GuardianApprovalRequest::WriteStdin { id: "terminal-open".to_string(), approval_id: "terminal-write".to_string(), + environment_id: "windows-executor".to_string(), process_id: 1000, input: input.to_string(), cwd: cwd.clone(), @@ -1381,6 +1382,7 @@ fn guardian_write_stdin_preserves_input_and_foreign_cwd() -> serde_json::Result< guardian_approval_request_to_json(&action)?, serde_json::json!({ "tool": "write_stdin", + "environment_id": "windows-executor", "session_id": 1000, "chars": input, "cwd": r"C:\workspace", diff --git a/codex-rs/core/src/session/elicitation_holders_tests.rs b/codex-rs/core/src/session/elicitation_holders_tests.rs index 3d0b09cbf7..2f85db1eec 100644 --- a/codex-rs/core/src/session/elicitation_holders_tests.rs +++ b/codex-rs/core/src/session/elicitation_holders_tests.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use codex_core_plugins::PluginCommandAttribution; use codex_plugin::PluginId; +use codex_protocol::approvals::ExecApprovalKind; use codex_protocol::protocol::ReviewDecision; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; @@ -50,11 +51,12 @@ async fn command_approval_holds_an_elicitation_until_response() { session .request_command_approval( turn_context.as_ref(), + ExecApprovalKind::Command, "call-1".to_string(), /*approval_id*/ None, /*environment_id*/ None, vec!["echo".to_string()], - cwd, + cwd.into(), /*reason*/ None, /*network_approval_context*/ None, /*proposed_execpolicy_amendment*/ None, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index d898ff969d..c48c016b61 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2475,11 +2475,12 @@ impl Session { pub async fn request_command_approval( &self, turn_context: &TurnContext, + kind: ExecApprovalKind, call_id: String, approval_id: Option, environment_id: Option, command: Vec, - cwd: AbsolutePathBuf, + cwd: PathUri, reason: Option, network_approval_context: Option, proposed_execpolicy_amendment: Option, @@ -2489,7 +2490,7 @@ impl Session { ) -> ReviewDecision { let _elicitation = self.services.elicitations.register(); // command-level approvals use `call_id`. - // `approval_id` is only present for subcommand callbacks (execve intercept) + // `approval_id` identifies subcommand callbacks and stdin writes. let effective_approval_id = approval_id.clone().unwrap_or_else(|| call_id.clone()); // Add the tx_approve callback to the map before sending the request. let (tx_approve, rx_approve) = oneshot::channel(); @@ -2528,14 +2529,17 @@ impl Session { additional_permissions.as_ref(), ) }); - let plugin_attribution = plugin_attribution_override - .or_else(|| turn_context.plugin_attribution_for_command(&command, &cwd)); + let plugin_attribution = plugin_attribution_override.or_else(|| { + cwd.to_abs_path() + .ok() + .and_then(|cwd| turn_context.plugin_attribution_for_command(&command, &cwd)) + }); let (plugin_id, script_path) = plugin_attribution .as_ref() .map(PluginCommandAttribution::serialized_fields) .unzip(); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { - kind: ExecApprovalKind::Command, + kind, call_id, plugin_id, script_path, @@ -2544,7 +2548,7 @@ impl Session { environment_id, started_at_ms: now_unix_timestamp_ms(), command, - cwd, + cwd: cwd.into(), reason, network_approval_context, proposed_execpolicy_amendment, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b51db867c0..88ddab0c66 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -8280,7 +8280,7 @@ async fn shutdown_and_wait_shuts_down_tracked_ephemeral_guardian_review() { .expect("ephemeral guardian review should receive a shutdown op"); } -async fn make_session_and_context_with_auth_and_config_and_rx( +pub(crate) async fn make_session_and_context_with_auth_and_config_and_rx( auth: CodexAuth, dynamic_tools: Vec, configure_config: F, diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 91a90cb6ca..8fb75d7360 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -1,6 +1,7 @@ //! Central approval policy-stage execution and reviewer routing. use crate::command_canonicalization::canonicalize_command_for_approval; +use crate::exec_policy::prompt_is_rejected_by_policy; use crate::guardian::GuardianNetworkAccessTrigger; use crate::guardian::GuardianReviewContext; use crate::guardian::GuardianReviewOptions; @@ -25,6 +26,7 @@ use codex_analytics::GuardianApprovalRequestSource; use codex_config::types::AppToolApproval; use codex_hooks::PermissionRequestDecision; use codex_otel::ToolDecisionSource; +use codex_protocol::approvals::ExecApprovalKind; use codex_protocol::approvals::ExecPolicyAmendment; #[cfg(unix)] use codex_protocol::approvals::GuardianCommandSource; @@ -75,6 +77,15 @@ pub(crate) enum ApprovalAction { tty: bool, proposed_execpolicy_amendment: Option, }, + WriteStdin { + id: String, + approval_id: String, + environment_id: String, + process_id: i32, + input: String, + cwd: PathUri, + tty: bool, + }, #[cfg(unix)] Execve { id: String, @@ -152,6 +163,26 @@ impl ApprovalAction { justification, .. } => PermissionRequestPayload::bash(hook_command.clone(), justification.clone()), + Self::WriteStdin { + id, + approval_id, + environment_id, + process_id, + input, + cwd, + tty, + } => PermissionRequestPayload { + tool_name: HookToolName::new("write_stdin"), + tool_input: serde_json::json!({ + "session_id": process_id, + "chars": input, + "parent_call_id": id, + "approval_id": approval_id, + "environment_id": environment_id, + "cwd": cwd, + "tty": tty, + }), + }, #[cfg(unix)] Self::Execve { command, .. } => PermissionRequestPayload::bash( codex_shell_command::parse_command::shlex_join(command), @@ -216,7 +247,8 @@ impl ApprovalAction { Self::Execve { .. } => Vec::new(), Self::McpToolCall { .. } | Self::NetworkAccess { .. } - | Self::RequestPermissions { .. } => Vec::new(), + | Self::RequestPermissions { .. } + | Self::WriteStdin { .. } => Vec::new(), Self::ApplyPatch { environment_id, files, @@ -255,6 +287,23 @@ impl ApprovalAction { justification, tty, }, + Self::WriteStdin { + id, + approval_id, + environment_id, + process_id, + input, + cwd, + tty, + } => crate::guardian::GuardianApprovalRequest::WriteStdin { + id, + approval_id, + environment_id, + process_id, + input, + cwd, + tty, + }, #[cfg(unix)] Self::Execve { id, @@ -436,6 +485,16 @@ impl Session { action: ApprovalAction, ctx: ApprovalContext, ) -> Result { + // Stdin is a fresh sandbox approval, not an exec-policy cache hit. Strict + // review can route Never to Guardian, but granular restrictions still apply. + let policy = ctx.review_context.turn().approval_policy(); + if matches!(&action, ApprovalAction::WriteStdin { .. }) + && !(ctx.strict_auto_review && matches!(policy, AskForApproval::Never)) + && let Some(reason) = + prompt_is_rejected_by_policy(policy, /*prompt_is_rule*/ false) + { + return Err(ToolError::Rejected(reason.to_string())); + } let is_mcp_tool_call = matches!(&action, ApprovalAction::McpToolCall { .. }); let is_network_approval = matches!(&action, ApprovalAction::NetworkAccess { .. }); let permission_request_run_id = match &action { @@ -532,6 +591,20 @@ impl Session { action: ApprovalAction, ctx: &ApprovalContext, ) -> ReviewDecision { + // Guardian inherits only the current turn's ready environments. A retained + // terminal handle may outlive its selection, but must not be reviewed in + // a different environment that happens to have the same launch directory. + if let ApprovalAction::WriteStdin { environment_id, .. } = &action + && !ctx + .review_context + .environments() + .turn_environments() + .any(|environment| environment.selection.environment_id == *environment_id) + { + return ReviewDecision::denied( + "automatic approval review cannot access the terminal's environment; select it before retrying", + ); + } let is_network_approval = matches!(&action, ApprovalAction::NetworkAccess { .. }); let review_id = new_guardian_review_id(); let action = match action.into_guardian_request() { @@ -650,11 +723,12 @@ impl Session { with_cached_approval(&self.services, tool_name, cache_keys, || async { self.request_command_approval( ctx.review_context.turn(), + ExecApprovalKind::Command, ctx.call_id.clone(), /*approval_id*/ None, Some(environment_id.clone()), command.clone(), - cwd, + cwd.into(), reason, ctx.network_approval_context.clone(), proposed_execpolicy_amendment.clone(), @@ -666,6 +740,37 @@ impl Session { }) .await } + ApprovalAction::WriteStdin { + id, + approval_id, + environment_id, + process_id, + input, + cwd, + .. + } => { + self.request_command_approval( + ctx.review_context.turn(), + ExecApprovalKind::WriteStdin, + id.clone(), + Some(approval_id.clone()), + Some(environment_id.clone()), + vec![ + "write_stdin".to_string(), + "--session-id".to_string(), + process_id.to_string(), + input.clone(), + ], + cwd.clone(), + ctx.approval_reason.clone(), + /*network_approval_context*/ None, + /*proposed_execpolicy_amendment*/ None, + /*additional_permissions*/ None, + Some(vec![ReviewDecision::Approved, ReviewDecision::Abort]), + /*plugin_attribution_override*/ None, + ) + .await + } #[cfg(unix)] ApprovalAction::Execve { approval_id, @@ -677,11 +782,12 @@ impl Session { } => { self.request_command_approval( ctx.review_context.turn(), + ExecApprovalKind::Command, ctx.call_id.clone(), Some(approval_id.clone()), Some(environment_id.clone()), command.clone(), - cwd.clone(), + cwd.clone().into(), /*reason*/ None, /*network_approval_context*/ None, /*proposed_execpolicy_amendment*/ None, @@ -748,11 +854,12 @@ impl Session { } => { self.request_command_approval( ctx.review_context.turn(), + ExecApprovalKind::Command, ctx.call_id.clone(), /*approval_id*/ None, Some(environment_id.clone()), command.clone(), - cwd.clone(), + cwd.clone().into(), ctx.approval_reason.clone(), ctx.network_approval_context.clone(), /*proposed_execpolicy_amendment*/ None, diff --git a/codex-rs/core/src/tools/approvals_tests.rs b/codex-rs/core/src/tools/approvals_tests.rs index d5800e7846..10f05820f8 100644 --- a/codex-rs/core/src/tools/approvals_tests.rs +++ b/codex-rs/core/src/tools/approvals_tests.rs @@ -95,7 +95,6 @@ fn guardian_cwd_rejects_foreign_remote_path() { assert!(guardian_cwd(codex_exec_server::REMOTE_ENVIRONMENT_ID, cwd).is_err()); } - #[tokio::test] async fn explicit_mcp_reviewer_override_takes_precedence_over_action_context() { let (session, turn, events) = make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs index 23be88be68..2a8d3921f0 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/write_stdin.rs @@ -7,6 +7,9 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::PostToolUsePayload; use crate::tools::registry::PreToolUsePayload; use crate::tools::registry::ToolExecutor; +use crate::tools::sandboxing::ToolError; +use crate::unified_exec::UnifiedExecContext; +use crate::unified_exec::UnifiedExecError; use crate::unified_exec::WriteStdinInteractionEvent; use crate::unified_exec::WriteStdinRequest; use codex_tools::ToolName; @@ -56,6 +59,9 @@ impl WriteStdinHandler { let ToolInvocation { session, turn, + step_context, + cancellation_token, + call_id, payload, .. } = invocation; @@ -70,23 +76,37 @@ impl WriteStdinHandler { }; let args: WriteStdinArgs = parse_arguments(&arguments)?; + let context = + UnifiedExecContext::new(session.clone(), step_context, cancellation_token, call_id); let response = session .services .unified_exec_manager - .write_stdin(WriteStdinRequest { - process_id: args.session_id, - input: &args.chars, - yield_time_ms: args.yield_time_ms, - max_output_tokens: args.max_output_tokens, - truncation_policy: turn.model_info().truncation_policy.into(), - interaction_event: Some(WriteStdinInteractionEvent { - session: &session, - turn: &turn, - }), - }) + .write_stdin( + &context, + WriteStdinRequest { + process_id: args.session_id, + input: &args.chars, + yield_time_ms: args.yield_time_ms, + max_output_tokens: args.max_output_tokens, + truncation_policy: turn.model_info().truncation_policy.into(), + interaction_event: Some(WriteStdinInteractionEvent { + session: &session, + turn: &turn, + }), + }, + ) .await .map_err(|err| { - FunctionCallError::RespondToModel(format!("write_stdin failed: {err}")) + let message = match err { + UnifiedExecError::StdinApproval(ToolError::Rejected(reason)) => { + format!("write_stdin rejected: {reason}") + } + UnifiedExecError::StdinApproval(ToolError::Codex(err)) => { + format!("write_stdin approval failed: {err}") + } + err => format!("write_stdin failed: {err}"), + }; + FunctionCallError::RespondToModel(message) })?; Ok(boxed_tool_output(response)) diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index c1e1be80ab..1c89697568 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -106,6 +106,7 @@ pub struct UnifiedExecRuntime<'a> { pub(crate) struct UnifiedExecAttempt { pub(crate) process: UnifiedExecProcess, pub(crate) metrics_sidecar: Option, + pub(crate) escalated: bool, } fn unified_exec_options( @@ -475,6 +476,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt return Ok(UnifiedExecAttempt { process, metrics_sidecar, + escalated: attempt.is_escalated(), }); } None => { @@ -519,6 +521,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt Ok(UnifiedExecAttempt { process, metrics_sidecar, + escalated: attempt.is_escalated(), }) } } diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 274497009d..195cf54e96 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -418,6 +418,17 @@ pub(crate) fn executor_windows_sandbox_level( } impl<'a> SandboxAttempt<'a> { + /// Whether this attempt bypasses sandboxing required by its ambient policy. + /// Use the requested policy, not the controller's wrapper, for remote executors. + pub(crate) fn is_escalated(&self) -> bool { + !self.sandbox_requested + && self.manager.should_sandbox( + self.permissions, + SandboxablePreference::Auto, + self.enforce_managed_network, + ) + } + pub(crate) fn network_proxy<'b>( &'b self, fallback: Option<&'b NetworkProxy>, diff --git a/codex-rs/core/src/tools/sandboxing_tests.rs b/codex-rs/core/src/tools/sandboxing_tests.rs index 5aff20e706..ba63e270eb 100644 --- a/codex-rs/core/src/tools/sandboxing_tests.rs +++ b/codex-rs/core/src/tools/sandboxing_tests.rs @@ -352,6 +352,7 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { let request = attempt .env_for_exec_server(command(), options()) .expect("prepare remote exec request"); + assert!(!attempt.is_escalated()); assert_eq!( request.command, @@ -390,8 +391,15 @@ fn exec_server_env_keeps_command_native_and_carries_sandbox_context() { let request = attempt .env_for_exec_server(command(), options()) .expect("prepare unsandboxed remote exec request"); + assert!(attempt.is_escalated()); assert_eq!(request.exec_server_sandbox, None); assert!(!request.exec_server_enforce_managed_network); assert_eq!(request.exec_server_managed_network, Some(managed_network)); + + let full_access = codex_protocol::models::PermissionProfile::Disabled; + attempt.permissions = &full_access; + attempt.exec_server_permissions = &full_access; + attempt.enforce_managed_network = false; + assert!(!attempt.is_escalated(), "full access is not an escalation"); } diff --git a/codex-rs/core/src/unified_exec/errors.rs b/codex-rs/core/src/unified_exec/errors.rs index dd401046ae..8fa2ef7857 100644 --- a/codex-rs/core/src/unified_exec/errors.rs +++ b/codex-rs/core/src/unified_exec/errors.rs @@ -12,6 +12,8 @@ pub(crate) enum UnifiedExecError { // The model is trained on `session_id`, but internally we track a `process_id`. #[error("Unknown process id {process_id}")] UnknownProcessId { process_id: i32 }, + #[error("stdin approval failed: {0:?}")] + StdinApproval(crate::tools::sandboxing::ToolError), #[error("failed to write to stdin")] WriteToStdin, #[error( diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 3762b7f437..4f50e2dc4a 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -184,6 +184,10 @@ struct ProcessEntry { initial_exec_command_active: Arc, hook_command: String, tty: bool, + environment_id: String, + // The successful launch bypassed sandboxing required by its ambient policy. + // Preserve this across turns so subsequent stdin writes can require approval. + escalated: bool, network_approval: Option, session: Weak, last_used: tokio::time::Instant, diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index b4f352d015..f70caaa059 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -141,6 +141,8 @@ async fn exec_command_with_tty( initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(true)), hook_command: cmd.to_string(), tty, + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + escalated: false, network_approval: None, session: Arc::downgrade(session), last_used: started_at, @@ -304,6 +306,7 @@ async fn blocking_terminate_unified_process( async fn write_stdin( session: &Arc, + turn: &Arc, process_id: i32, input: &str, yield_time_ms: u64, @@ -311,14 +314,22 @@ async fn write_stdin( session .services .unified_exec_manager - .write_stdin(WriteStdinRequest { - process_id, - input, - yield_time_ms, - max_output_tokens: None, - truncation_policy: TruncationPolicy::Tokens(10_000), - interaction_event: None, - }) + .write_stdin( + &UnifiedExecContext::new( + Arc::clone(session), + crate::session::step_context::StepContext::for_test(Arc::clone(turn)), + tokio_util::sync::CancellationToken::new(), + "write".to_string(), + ), + WriteStdinRequest { + process_id, + input, + yield_time_ms, + max_output_tokens: None, + truncation_policy: TruncationPolicy::Tokens(10_000), + interaction_event: None, + }, + ) .await } @@ -347,6 +358,7 @@ async fn unified_exec_persists_across_requests() -> anyhow::Result<()> { write_stdin( &session, + &turn, process_id, "export CODEX_INTERACTIVE_SHELL_VAR=codex\n", /*yield_time_ms*/ 2_500, @@ -355,6 +367,7 @@ async fn unified_exec_persists_across_requests() -> anyhow::Result<()> { let out_2 = write_stdin( &session, + &turn, process_id, "echo $CODEX_INTERACTIVE_SHELL_VAR\n", /*yield_time_ms*/ 2_500, @@ -388,6 +401,7 @@ async fn multi_unified_exec_sessions() -> anyhow::Result<()> { write_stdin( &session, + &turn, session_a, "export CODEX_INTERACTIVE_SHELL_VAR=codex\n", /*yield_time_ms*/ 2_500, @@ -416,6 +430,7 @@ async fn multi_unified_exec_sessions() -> anyhow::Result<()> { let out_3 = write_stdin( &session, + &turn, shell_a.process_id.expect("expected process id"), "echo $CODEX_INTERACTIVE_SHELL_VAR\n", /*yield_time_ms*/ 2_500, @@ -447,6 +462,7 @@ async fn unified_exec_timeouts() -> anyhow::Result<()> { write_stdin( &session, + &turn, process_id, format!("export CODEX_INTERACTIVE_SHELL_VAR={TEST_VAR_VALUE}\n").as_str(), /*yield_time_ms*/ 2_500, @@ -455,6 +471,7 @@ async fn unified_exec_timeouts() -> anyhow::Result<()> { let out_2 = write_stdin( &session, + &turn, process_id, "sleep 5 && echo $CODEX_INTERACTIVE_SHELL_VAR\n", /*yield_time_ms*/ 10, @@ -469,7 +486,7 @@ async fn unified_exec_timeouts() -> anyhow::Result<()> { tokio::time::sleep(Duration::from_secs(7)).await; - let out_3 = write_stdin(&session, process_id, "", /*yield_time_ms*/ 100).await?; + let out_3 = write_stdin(&session, &turn, process_id, "", /*yield_time_ms*/ 100).await?; assert!( out_3 @@ -533,11 +550,14 @@ async fn reusing_completed_process_returns_unknown_process() -> anyhow::Result<( .await?; let process_id = open_shell.process_id.expect("expected process id"); - write_stdin(&session, process_id, "exit\n", /*yield_time_ms*/ 2_500).await?; + write_stdin( + &session, &turn, process_id, "exit\n", /*yield_time_ms*/ 2_500, + ) + .await?; tokio::time::sleep(Duration::from_millis(200)).await; - let err = write_stdin(&session, process_id, "", /*yield_time_ms*/ 100) + let err = write_stdin(&session, &turn, process_id, "", /*yield_time_ms*/ 100) .await .expect_err("expected unknown process error"); @@ -588,6 +608,8 @@ async fn terminating_initial_exec_command_rechecks_initial_response_state() -> a initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(true)), hook_command: "sleep 60".to_string(), tty: true, + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + escalated: false, network_approval: None, session: Arc::downgrade(&session), last_used: Instant::now(), @@ -662,6 +684,8 @@ async fn terminating_during_stdin_poll_returns_exited_response() -> anyhow::Resu initial_exec_command_active: Arc::new(std::sync::atomic::AtomicBool::new(false)), hook_command: "sleep 60".to_string(), tty: true, + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + escalated: false, network_approval: None, session: Arc::downgrade(&session), last_used, @@ -670,8 +694,12 @@ async fn terminating_during_stdin_poll_returns_exited_response() -> anyhow::Resu let poll_task = tokio::spawn({ let session = Arc::clone(&session); + let turn = Arc::clone(&turn); async move { - write_stdin(&session, process_id, "", /*yield_time_ms*/ 60_000).await + write_stdin( + &session, &turn, process_id, "", /*yield_time_ms*/ 60_000, + ) + .await } }); tokio::time::timeout(Duration::from_secs(2), async { @@ -835,3 +863,121 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() ); Ok(()) } + +#[tokio::test] +async fn stdin_approval_preserves_the_reviewed_terminal() -> anyhow::Result<()> { + use crate::session::tests::make_session_and_context_with_auth_and_config_and_rx; + use crate::state::ActiveTurn; + use crate::tools::sandboxing::ToolError; + use codex_features::Feature; + use codex_protocol::config_types::ApprovalsReviewer; + use codex_protocol::protocol::AskForApproval; + use codex_protocol::protocol::EventMsg; + use codex_protocol::protocol::ReviewDecision; + + skip_if_sandbox!(Ok(())); + let (session, mut turn, events) = make_session_and_context_with_auth_and_config_and_rx( + codex_login::CodexAuth::from_api_key("Test API Key"), + Vec::new(), + |config| { + config.features.enable(Feature::WriteStdinApproval).unwrap(); + config.permissions.approval_policy = + crate::config::Constrained::allow_any(AskForApproval::OnRequest); + config.approvals_reviewer = ApprovalsReviewer::AutoReview; + }, + ) + .await; + *session.active_turn.lock().await = Some(ActiveTurn::default()); + let manager = &session.services.unified_exec_manager; + let command = "while IFS= read -r line; do printf 'received:%s\\n' \"$line\"; done"; + let opened = exec_command( + &session, &turn, command, /*yield_time_ms*/ 250, /*workdir*/ None, + ) + .await?; + let process_id = opened.process_id.expect("running terminal"); + let cwd = PathUri::parse("file:///C:/workspace")?; + let original = { + let mut store = manager.process_store.lock().await; + let entry = store.processes.get_mut(&process_id).unwrap(); + entry.escalated = true; + entry.environment_id = "unselected-executor".to_string(); + entry.cwd = cwd.clone(); + Arc::clone(&entry.process) + }; + // Empty polling must complete without an approval response. + tokio::time::timeout( + Duration::from_secs(/*secs*/ 5), + write_stdin(&session, &turn, process_id, "", /*yield_time_ms*/ 250), + ) + .await??; + let input = "rejected\n"; + let denied = write_stdin( + &session, &turn, process_id, input, /*yield_time_ms*/ 250, + ) + .await; + assert!( + matches!(denied, Err(UnifiedExecError::StdinApproval(ToolError::Rejected(reason))) + if reason.contains("select it before retrying")) + ); + Arc::make_mut(&mut Arc::get_mut(&mut turn).unwrap().config).approvals_reviewer = + ApprovalsReviewer::User; + for (input, decision) in [ + ("rejected\n", ReviewDecision::denied("test denial")), + ("accepted\n", ReviewDecision::Approved), + ("replace\n", ReviewDecision::Approved), + ] { + let review = async { + loop { + let EventMsg::ExecApprovalRequest(approval) = events.recv().await?.msg else { + continue; + }; + assert_eq!( + ( + approval.call_id.as_str(), + approval.approval_id.as_deref(), + approval.cwd + ), + ("call", Some("write"), cwd.clone().into()) + ); + assert!(original.interaction_lock().try_lock_owned().is_err()); + if input == "replace\n" { + let replacement = super::process_tests::remote_process( + WriteStatus::Accepted, + /*terminate_error*/ None, + SandboxType::None, + ) + .await; + let mut store = manager.process_store.lock().await; + store.processes.get_mut(&process_id).unwrap().process = Arc::new(replacement); + } + session.notify_approval("write", decision).await; + return anyhow::Ok(()); + } + }; + let (result, reviewed) = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), async { + tokio::join!( + write_stdin( + &session, &turn, process_id, input, /*yield_time_ms*/ 250 + ), + review + ) + }) + .await?; + reviewed?; + match input { + "rejected\n" => assert!(matches!(result, Err(UnifiedExecError::StdinApproval(_)))), + "accepted\n" => { + let output = result?.truncated_output(DEFAULT_MAX_OUTPUT_TOKENS); + assert!(output.contains("received:accepted"), "{output}"); + assert!(!output.contains("received:rejected"), "{output}"); + } + _ => assert!( + matches!(result, Err(UnifiedExecError::UnknownProcessId { process_id: id }) if id == process_id) + ), + } + assert!(original.interaction_lock().try_lock_owned().is_ok()); + } + original.terminate(); + assert!(session.terminate_background_terminal(process_id).await); + Ok(()) +} diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 1bbd73b7a2..2558833b01 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -20,10 +20,12 @@ use crate::exec_env::inject_apply_patch_env; use crate::exec_env::inject_permission_profile_env; use crate::exec_env::inject_session_id_env; use crate::exec_policy::ExecApprovalRequest; +use crate::guardian::GuardianReviewContext; use crate::plugins::metrics::finish_and_track_measurements; use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecRequest; use crate::sandboxing::ExecServerEnvConfig; +use crate::tools::ApprovalContext; use crate::tools::context::ExecCommandToolOutput; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; @@ -35,6 +37,7 @@ use crate::tools::runtimes::is_managed_proxy_env_var; use crate::tools::runtimes::unified_exec::UnifiedExecAttempt; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; +use crate::tools::sandboxing::ApprovalAction; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; @@ -66,6 +69,7 @@ use codex_core_plugins::PLUGIN_METRICS_OUTPUT_ENV_VAR; use codex_core_plugins::PluginCommandAttribution; use codex_core_plugins::PluginMetricsSidecar; use codex_core_plugins::strip_output_env; +use codex_features::Feature; use codex_network_proxy::NetworkPolicyDecider; use codex_network_proxy::NetworkProxy; use codex_protocol::config_types::ShellEnvironmentPolicy; @@ -485,6 +489,7 @@ impl UnifiedExecProcessManager { let UnifiedExecAttempt { process, metrics_sidecar, + escalated, } = attempt; let process = Arc::new(process); let network_denial_monitor = deferred_network_approval.as_ref().map(|deferred| { @@ -543,6 +548,8 @@ impl UnifiedExecProcessManager { &request.command, request.hook_command.clone(), cwd.clone(), + request.turn_environment.selection.environment_id.clone(), + escalated, plugin_attribution.clone(), start, request.process_id, @@ -757,6 +764,7 @@ impl UnifiedExecProcessManager { pub(crate) async fn write_stdin( &self, + context: &UnifiedExecContext, request: WriteStdinRequest<'_>, ) -> Result { let process_id = request.process_id; @@ -764,16 +772,55 @@ impl UnifiedExecProcessManager { // Different terminal sessions can be polled concurrently, but reads and // writes against one terminal must not overlap because they share a // draining output buffer and process lifecycle. - let locked_process = { + let (locked_process, approval) = { let store = self.process_store.lock().await; let entry = store .processes .get(&process_id) .ok_or(UnifiedExecError::UnknownProcessId { process_id })?; - Arc::clone(&entry.process) + let approval = (!request.input.is_empty() + && (entry.tty || request.input != INTERRUPT) + && entry.escalated + && context + .session + .features() + .enabled(Feature::WriteStdinApproval)) + .then(|| ApprovalAction::WriteStdin { + id: entry.call_id.clone(), + approval_id: context.call_id.clone(), + environment_id: entry.environment_id.clone(), + process_id, + input: request.input.to_string(), + cwd: entry.cwd.clone(), + tty: entry.tty, + }); + (Arc::clone(&entry.process), approval) }; let _interaction_guard = locked_process.interaction_lock().lock_owned().await; + if let Some(approval) = approval { + let strict_auto_review = context + .session + .active_turn_context_and_strict_auto_review() + .await + .is_some_and(|(_, _, strict)| strict); + let approval_context = ApprovalContext { + review_context: GuardianReviewContext::from(&context.step_context), + cancellation_token: Some(context.cancellation_token.clone()), + call_id: context.call_id.clone(), + tool_name: ToolName::plain("write_stdin"), + strict_auto_review, + approval_reason: Some("Send input to an existing escalated terminal. The cwd is its launch directory; the terminal's current directory and state may have changed.".to_string()), + retry_reason: None, + network_approval_context: None, + }; + context + .session + .request_approval(approval, approval_context) + .await + .map_err(UnifiedExecError::StdinApproval)?; + } + // Revalidate the identity after approval: a removed process ID can be reused. let PreparedProcessHandles { process, output, @@ -1003,6 +1050,8 @@ impl UnifiedExecProcessManager { command: &[String], hook_command: String, cwd: PathUri, + environment_id: String, + escalated: bool, plugin_attribution: Option, started_at: Instant, process_id: i32, @@ -1024,6 +1073,8 @@ impl UnifiedExecProcessManager { initial_exec_command_active, hook_command, tty, + environment_id, + escalated, network_approval, session: Arc::downgrade(&context.session), last_used: started_at, diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index a73db81887..5db327ecea 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -589,6 +589,8 @@ async fn pruning_does_not_evict_live_process_while_exited_process_is_finalizing( initial_exec_command_active: Arc::new(AtomicBool::new(false)), hook_command: format!("command-{process_id}"), tty: false, + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + escalated: false, network_approval: None, session: std::sync::Weak::new(), last_used: if is_exited { diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index 8414303c88..5651c6f825 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -887,7 +887,7 @@ async fn user_network_approval_once_session_and_denial_semantics() -> Result<()> assert_eq!(approval.approval_id.as_deref(), None); let first_approval_call_id = approval.call_id.clone(); assert!(!approval.turn_id.is_empty()); - assert_eq!(approval.cwd, test.config.cwd); + assert_eq!(approval.cwd, test.config.cwd.clone().into()); assert_eq!( approval.reason.as_deref(), Some("codex-network-test.invalid is not in the allowed_domains") @@ -1387,7 +1387,7 @@ async fn unattributed_network_request_uses_active_turn_environment_fallback() -> let proxy_request = tokio::spawn(raw_http_proxy_request(proxy_addr, NETWORK_TEST_HOST)); let approval = expect_network_approval(&test, LOCAL_ENVIRONMENT_ID).await?; assert_eq!(approval.command, ["network-access", NETWORK_TEST_TARGET]); - assert_eq!(approval.cwd, test.config.cwd); + assert_eq!(approval.cwd, test.config.cwd.clone().into()); test.codex .submit(Op::ExecApproval { id: approval.effective_approval_id(), diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index 0466fd18aa..eb32aa9794 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -1,6 +1,10 @@ use codex_core::EnvironmentConfig; use codex_core::TurnInputRequest; use codex_core::windows_sandbox::WindowsSandboxLevelExt; +use codex_features::Feature; +use codex_protocol::approvals::ExecApprovalKind; +use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::SandboxPolicy; use core_test_support::test_codex::local_selections; use std::collections::HashMap; use std::ffi::OsStr; @@ -1347,8 +1351,12 @@ async fn wait_for_unified_exec_end( (end_event, turn_completed) } +#[test_case::test_case(false; "without_stdin_approval")] +#[test_case::test_case(true; "with_stdin_approval")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()> { +async fn unified_exec_emits_terminal_interaction_for_write_stdin( + stdin_approval: bool, +) -> Result<()> { // TODO(anp): Remove after unified-exec interactive fixtures support Windows/ConPTY. skip_if_target_windows!(Ok(()), "uses POSIX interactive-process and EOF semantics"); skip_if_no_network!(Ok(())); @@ -1356,15 +1364,25 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()> let server = start_mock_server().await; - let mut builder = test_codex(); + let mut builder = test_codex().with_config(move |config| { + if stdin_approval { + config + .features + .enable(Feature::WriteStdinApproval) + .expect("enable stdin approvals"); + } + }); let test = builder.build_with_auto_env(&server).await?; let open_call_id = "uexec-open"; - let open_args = json!({ + let mut open_args = json!({ "cmd": "/bin/bash -i", "yield_time_ms": 200, "tty": true, }); + if stdin_approval { + open_args["sandbox_permissions"] = json!("require_escalated"); + } let stdin_call_id = "uexec-stdin-delta"; let stdin_args = json!({ @@ -1400,13 +1418,43 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()> ]; mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn(&test, "stdin delta", PermissionProfile::Disabled).await?; + if stdin_approval { + // Start without waiting for completion so the loop below can answer approvals. + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "stdin delta".to_string(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + approval_policy: Some(AskForApproval::OnRequest), + sandbox_policy: Some(SandboxPolicy::ReadOnly { + network_access: false, + }), + ..Default::default() + }), + ) + .await?; + } else { + submit_unified_exec_turn(&test, "stdin delta", PermissionProfile::Disabled).await?; + } let mut terminal_interaction = None; + let mut approvals = Vec::new(); loop { let msg = wait_for_event(&test.codex, |_| true).await; match msg { + EventMsg::ExecApprovalRequest(approval) => { + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: Some(approval.turn_id), + decision: ReviewDecision::Approved, + }) + .await?; + approvals.push((approval.kind, approval.call_id, approval.approval_id)); + } EventMsg::TerminalInteraction(ev) if ev.call_id == open_call_id => { terminal_interaction = Some(ev); } @@ -1415,6 +1463,21 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()> } } + assert_eq!( + approvals, + if stdin_approval { + vec![ + (ExecApprovalKind::Command, open_call_id.to_string(), None), + ( + ExecApprovalKind::WriteStdin, + open_call_id.to_string(), + Some(stdin_call_id.to_string()), + ), + ] + } else { + Vec::new() + } + ); let delta = terminal_interaction.expect("expected TerminalInteraction event"); assert_eq!(delta.process_id, "1000"); let expected_stdin = stdin_args @@ -2370,7 +2433,12 @@ async fn assert_write_stdin_ctrl_c_interrupts_non_tty_session( let server = start_mock_server().await; - let mut builder = test_codex(); + let mut builder = test_codex().with_config(|config| { + config + .features + .enable(Feature::WriteStdinApproval) + .expect("enable stdin approvals"); + }); let test = builder.build_with_auto_env(&server).await?; let start_call_id = format!("uexec-non-tty-interrupt-{test_name}-start"); @@ -2380,6 +2448,7 @@ async fn assert_write_stdin_ctrl_c_interrupts_non_tty_session( "cmd": command, "yield_time_ms": 250, "tty": false, + "sandbox_permissions": "require_escalated", }); let interrupt_args = serde_json::json!({ "chars": "\u{3}", @@ -2413,17 +2482,43 @@ async fn assert_write_stdin_ctrl_c_interrupts_non_tty_session( ]; let request_log = mount_sse_sequence(&server, responses).await; - submit_unified_exec_turn( - &test, - "interrupt non-tty unified exec", - PermissionProfile::Disabled, - ) - .await?; + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "interrupt non-tty unified exec".to_string(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + approval_policy: Some(AskForApproval::OnRequest), + sandbox_policy: Some(SandboxPolicy::ReadOnly { + network_access: false, + }), + ..Default::default() + }), + ) + .await?; - wait_for_event(&test.codex, |event| { - matches!(event, EventMsg::TurnComplete(_)) - }) - .await; + let mut approval_count = 0; + loop { + match wait_for_event(&test.codex, |_| true).await { + EventMsg::ExecApprovalRequest(approval) => { + approval_count += 1; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: Some(approval.turn_id), + decision: ReviewDecision::Approved, + }) + .await?; + } + EventMsg::TurnComplete(_) => break, + _ => {} + } + } + assert_eq!( + approval_count, 1, + "only process launch should need approval" + ); let requests = request_log.requests(); assert!(!requests.is_empty(), "expected at least one POST request"); diff --git a/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs b/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs index 218e228093..cbef67fbb0 100644 --- a/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs +++ b/codex-rs/core/tests/suite/unified_exec_zsh_fork_approvals.rs @@ -5,6 +5,7 @@ use codex_config::permissions_toml::PermissionProfileToml; use codex_config::types::ApprovalsReviewer; use codex_core::TurnInputRequest; use codex_core::sandboxing::SandboxPermissions; +use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -40,9 +41,9 @@ use core_test_support::test_codex::local_selections; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; -use core_test_support::zsh_fork::build_zsh_fork_test; use core_test_support::zsh_fork::restrictive_workspace_write_profile; use core_test_support::zsh_fork::zsh_fork_runtime; +use core_test_support::zsh_fork::zsh_fork_test_builder; use pretty_assertions::assert_eq; use regex_lite::Regex; use serde_json::Value; @@ -517,9 +518,14 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t ev_completed("resp-cross-turn-write"), ]), sse(vec![ - ev_response_created("resp-cross-turn-guardian"), - ev_assistant_message("msg-cross-turn-guardian", r#"{"outcome":"allow"}"#), - ev_completed("resp-cross-turn-guardian"), + ev_response_created("resp-cross-turn-write-guardian"), + ev_assistant_message("msg-cross-turn-write-guardian", r#"{"outcome":"allow"}"#), + ev_completed("resp-cross-turn-write-guardian"), + ]), + sse(vec![ + ev_response_created("resp-cross-turn-execve-guardian"), + ev_assistant_message("msg-cross-turn-execve-guardian", r#"{"outcome":"allow"}"#), + ev_completed("resp-cross-turn-execve-guardian"), ]), sse(vec![ ev_response_created("resp-cross-turn-second-done"), @@ -555,6 +561,7 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t .await?; let mut current_turn_id = None; + let mut stdin_assessment = None; let mut intercepted_assessment = None; loop { let event = tokio::time::timeout(Duration::from_secs(30), test.codex.next_event()) @@ -562,6 +569,16 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t .context("timed out waiting for current-turn intercepted execve Guardian review")??; match event.msg { EventMsg::TurnStarted(started) => current_turn_id = Some(started.turn_id), + EventMsg::GuardianAssessment(assessment) + if assessment.status == GuardianAssessmentStatus::Approved + && matches!( + &assessment.action, + GuardianAssessmentAction::WriteStdin { approval_id, .. } + if approval_id == write_call_id + ) => + { + stdin_assessment = Some(assessment); + } EventMsg::GuardianAssessment(assessment) if assessment.status == GuardianAssessmentStatus::Approved && matches!(assessment.action, GuardianAssessmentAction::Execve { .. }) => @@ -578,6 +595,15 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t let current_turn_id = current_turn_id.context("expected the second turn to start")?; assert_ne!(current_turn_id, first_completion.turn_id); + let stdin_assessment = + stdin_assessment.context("expected an approved Guardian assessment for stdin")?; + assert_eq!( + ( + stdin_assessment.turn_id.as_str(), + stdin_assessment.target_item_id.as_deref(), + ), + (current_turn_id.as_str(), Some(open_call_id)), + ); let assessment = intercepted_assessment .context("expected an approved Guardian assessment for the persistent terminal")?; assert_eq!(assessment.turn_id, current_turn_id); @@ -594,8 +620,13 @@ async fn unified_exec_zsh_fork_guardian_reviews_persistent_terminal_in_current_t request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian") }) .collect::>(); - assert_eq!(guardian_requests.len(), 1); + assert_eq!(guardian_requests.len(), 2); assert!(guardian_requests[0].body_contains_text(&outside_path.to_string_lossy())); + let environment_id = &test.executor_environment().selection().environment_id; + let environment = format!(r#""environment_id": "{environment_id}""#); + assert!(guardian_requests[0].body_contains_text(&environment)); + assert!(guardian_requests[0].body_contains_text("The `cwd` field is its launch directory")); + assert!(guardian_requests[1].body_contains_text(&outside_path.to_string_lossy())); Ok(()) } @@ -619,14 +650,17 @@ where }; let server = start_mock_server().await; - let test = build_zsh_fork_test( - &server, - runtime, - approval_policy, - permission_profile, - pre_build_hook, - ) - .await?; + let test = zsh_fork_test_builder(runtime, approval_policy) + .with_pre_build_hook(pre_build_hook) + .with_config(move |config| { + config + .permissions + .set_permission_profile(permission_profile) + .expect("set permission profile"); + assert!(config.features.enable(Feature::WriteStdinApproval).is_ok()); + }) + .build(&server) + .await?; Ok(Some((server, test))) } diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 80cc9fac25..3a520f0198 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -137,6 +137,8 @@ pub enum Feature { ApplyPatchPreserveLineEndings, /// Allow exec tools to request additional permissions while staying sandboxed. ExecPermissionApprovals, + /// Require approval before writing input to escalated unified-exec terminals. + WriteStdinApproval, /// Expose the built-in request_permissions tool. RequestPermissionsTool, /// Allow the model to request web searches that fetch live content. @@ -1079,6 +1081,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::WriteStdinApproval, + key: "write_stdin_approval", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::CodexHooks, key: "hooks", diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index a097b7800e..e96526deb5 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -217,7 +217,7 @@ async fn run_codex_tool_session_inner( } = ev; handle_exec_approval_request( command, - cwd.to_path_buf(), + std::path::PathBuf::from(cwd.into_string()), outgoing.clone(), thread.clone(), request_id.clone(), diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 0dd621ff41..e4dedf5347 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -6,6 +6,7 @@ use crate::protocol::FileChange; use crate::protocol::ReviewDecision; use crate::request_permissions::RequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; @@ -152,6 +153,7 @@ pub enum GuardianAssessmentAction { approval_id: String, process_id: String, stdin: String, + /// Launch directory of the existing terminal, not its current working directory. cwd: PathUri, }, ApplyPatch { @@ -279,8 +281,8 @@ pub struct ExecApprovalRequestEvent { pub started_at_ms: i64, /// The command to be executed. pub command: Vec, - /// The command's working directory. - pub cwd: AbsolutePathBuf, + /// The command's working directory, or the launch directory for terminal input. + pub cwd: LegacyAppPathString, /// Optional human-readable reason for the approval (e.g. retry without sandbox). #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index e253ce8ef5..8b5174f0d7 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -220,6 +220,7 @@ impl App { let proposed_network_policy_amendments = params.proposed_network_policy_amendments.clone(); let approval = ExecApprovalRequest { + kind: params.kind, thread_id, thread_label, id: params diff --git a/codex-rs/tui/src/approval_events.rs b/codex-rs/tui/src/approval_events.rs index ba183225b8..ea8146b90f 100644 --- a/codex-rs/tui/src/approval_events.rs +++ b/codex-rs/tui/src/approval_events.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; use crate::diff_model::FileChange; use codex_app_server_protocol::AdditionalPermissionProfile; use codex_app_server_protocol::CommandExecutionApprovalDecision; +use codex_app_server_protocol::CommandExecutionApprovalKind; use codex_app_server_protocol::ExecPolicyAmendment; use codex_app_server_protocol::NetworkApprovalContext; use codex_app_server_protocol::NetworkPolicyAmendment; @@ -21,6 +22,8 @@ use serde::Serialize; #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct ExecApprovalRequestEvent { + #[serde(default)] + pub(crate) kind: CommandExecutionApprovalKind, pub(crate) call_id: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub(crate) approval_id: Option, diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index c561dde317..836cd1dba5 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -40,6 +40,7 @@ use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; use codex_app_server_protocol::AdditionalPermissionProfile; use codex_app_server_protocol::CommandExecutionApprovalDecision; +use codex_app_server_protocol::CommandExecutionApprovalKind; use codex_app_server_protocol::FileChangeApprovalDecision; use codex_app_server_protocol::FileSystemAccessMode; use codex_app_server_protocol::FileSystemPath; @@ -79,6 +80,7 @@ pub(crate) enum ApprovalRequest { #[derive(Clone, Debug)] pub(crate) struct ExecApprovalRequest { + pub kind: CommandExecutionApprovalKind, pub thread_id: ThreadId, pub thread_label: Option, pub id: String, @@ -250,23 +252,35 @@ impl ApprovalOverlay { list_keymap: &ListKeymap, ) -> (Vec, SelectionViewParams) { let (options, title) = match request { - ApprovalRequest::Exec(request) => ( - exec_options( - &request.available_decisions, - request.network_approval_context.as_ref(), - request.additional_permissions.as_ref(), - approval_keymap, - ), - request.network_approval_context.as_ref().map_or_else( - || "Would you like to run the following command?".to_string(), - |network_approval_context| { - format!( - "Do you want to approve network access to \"{}\"?", - network_approval_context.host - ) - }, - ), - ), + ApprovalRequest::Exec(request) => { + let title = if request.kind == CommandExecutionApprovalKind::WriteStdin { + request.command.get(2).map_or_else( + || "Would you like to send input to the existing terminal?".to_string(), + |process_id| { + format!("Would you like to send input to terminal {process_id}?") + }, + ) + } else { + request.network_approval_context.as_ref().map_or_else( + || "Would you like to run the following command?".to_string(), + |network_approval_context| { + format!( + "Do you want to approve network access to \"{}\"?", + network_approval_context.host + ) + }, + ) + }; + ( + exec_options( + &request.available_decisions, + request.network_approval_context.as_ref(), + request.additional_permissions.as_ref(), + approval_keymap, + ), + title, + ) + } ApprovalRequest::Permissions(_) => ( permissions_options(approval_keymap), "Would you like to grant these permissions?".to_string(), @@ -704,13 +718,18 @@ fn build_header(request: &ApprovalRequest) -> Box { ])); header.push(Line::from("")); } - let full_cmd = strip_bash_lc_and_escape(&request.command); - let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd); - if let Some(first) = full_cmd_lines.first_mut() { - first.spans.insert(0, Span::from("$ ")); - } - if request.network_approval_context.is_none() { - header.extend(full_cmd_lines); + if request.kind == CommandExecutionApprovalKind::WriteStdin { + let input = request.command.last().map_or("", String::as_str); + header.push(vec!["Input: ".into(), format!("{input:?}").into()].into()); + } else { + let full_cmd = strip_bash_lc_and_escape(&request.command); + let mut full_cmd_lines = highlight_bash_to_lines(&full_cmd); + if let Some(first) = full_cmd_lines.first_mut() { + first.spans.insert(0, Span::from("$ ")); + } + if request.network_approval_context.is_none() { + header.extend(full_cmd_lines); + } } Box::new(Paragraph::new(header).wrap(Wrap { trim: false })) } @@ -1187,6 +1206,7 @@ mod tests { fn make_exec_request() -> ApprovalRequest { ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1380,6 +1400,7 @@ mod tests { let tx = AppEventSender::new(tx); let mut view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1424,6 +1445,7 @@ mod tests { }; let mut view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1509,6 +1531,7 @@ mod tests { let thread_id = ThreadId::new(); let mut view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id, thread_label: Some("Robie [explorer]".to_string()), id: "test".to_string(), @@ -1544,6 +1567,7 @@ mod tests { keymap.approval.open_thread = vec![key_hint::plain(KeyCode::Char('x'))]; let mut view = make_overlay_with_keymap( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id, thread_label: Some("Robie [explorer]".to_string()), id: "test".to_string(), @@ -1583,6 +1607,7 @@ mod tests { let tx = AppEventSender::new(tx); let view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: Some("Robie [explorer]".to_string()), id: "test".to_string(), @@ -1612,6 +1637,7 @@ mod tests { let tx = AppEventSender::new(tx); let mut view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1665,6 +1691,7 @@ mod tests { let tx = AppEventSender::new(tx); let mut view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".to_string(), @@ -1705,6 +1732,7 @@ mod tests { let tx = AppEventSender::new(tx); let command = vec!["echo".into(), "hello".into(), "world".into()]; let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2006,6 +2034,7 @@ mod tests { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2063,6 +2092,7 @@ mod tests { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2188,6 +2218,7 @@ mod tests { let (tx, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx); let exec_request = ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".into(), @@ -2325,6 +2356,7 @@ mod tests { let tx = AppEventSender::new(tx_raw); let mut view = make_overlay( ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: ThreadId::new(), thread_label: None, id: "test".into(), diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index ded10d319a..592c835e20 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -2011,6 +2011,7 @@ mod tests { fn exec_request() -> ApprovalRequest { ApprovalRequest::Exec(ExecApprovalRequest { + kind: Default::default(), thread_id: codex_protocol::ThreadId::new(), thread_label: None, id: "1".to_string(), diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 4fee2ad703..b5215116e6 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -869,6 +869,7 @@ fn exec_approval_request_from_params( .and_then(|cwd| cwd.to_inferred_abs_path()) .unwrap_or_else(|| fallback_cwd.clone()); ExecApprovalRequestEvent { + kind: params.kind, call_id: params.item_id, command: params .command diff --git a/codex-rs/tui/src/chatwidget/interrupts.rs b/codex-rs/tui/src/chatwidget/interrupts.rs index 35a9cb56d9..951077dd9b 100644 --- a/codex-rs/tui/src/chatwidget/interrupts.rs +++ b/codex-rs/tui/src/chatwidget/interrupts.rs @@ -176,6 +176,7 @@ mod tests { fn exec_approval(call_id: &str, approval_id: Option<&str>) -> ExecApprovalRequestEvent { ExecApprovalRequestEvent { + kind: Default::default(), call_id: call_id.to_string(), approval_id: approval_id.map(str::to_string), turn_id: "turn".to_string(), diff --git a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs index 6e6aa7d16b..2d33e50c74 100644 --- a/codex-rs/tui/src/chatwidget/tests/approval_requests.rs +++ b/codex-rs/tui/src/chatwidget/tests/approval_requests.rs @@ -10,6 +10,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { // Trigger an exec approval request with a short, single-line command. let ev = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-short".into(), approval_id: Some("call-short".into()), turn_id: "turn-short".into(), @@ -86,6 +87,42 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() { ); } +#[tokio::test] +async fn app_server_write_stdin_approval_renders_terminal_input() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let event = exec_approval_request_from_params( + AppServerCommandExecutionRequestApprovalParams { + kind: codex_app_server_protocol::CommandExecutionApprovalKind::WriteStdin, + thread_id: "thread-1".to_string(), + turn_id: "turn-1".to_string(), + item_id: "item-1".to_string(), + started_at_ms: 0, + approval_id: Some("approval-1".to_string()), + environment_id: None, + reason: None, + network_approval_context: None, + command: Some( + shlex::try_join(["write_stdin", "--session-id", "42", "confirm\n"]) + .expect("round-trippable write_stdin input"), + ), + cwd: Some(test_path_buf("/tmp").abs().into()), + command_actions: None, + additional_permissions: None, + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, + available_decisions: None, + }, + &test_path_buf("/tmp").abs(), + ); + + handle_exec_approval_request(&mut chat, "approval-1", event); + + let area = Rect::new(0, 0, 80, chat.desired_height(/*width*/ 80)); + let mut buf = ratatui::buffer::Buffer::empty(area); + chat.render(area, &mut buf); + assert_snapshot!("write_stdin_approval_modal", format!("{buf:?}")); +} + #[test] fn app_server_exec_approval_request_preserves_permissions_context() { let read_path = AbsolutePathBuf::try_from(PathBuf::from(test_path_display("/tmp/read-only"))) @@ -338,6 +375,7 @@ async fn exec_approval_uses_approval_id_when_present() { &mut chat, "sub-short", ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-parent".into(), approval_id: Some("approval-subcommand".into()), turn_id: "turn-short".into(), @@ -381,6 +419,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let ev_multi = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-multi".into(), approval_id: Some("call-multi".into()), turn_id: "turn-multi".into(), @@ -433,6 +472,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { let long = format!("echo {}", "a".repeat(200)); let ev_long = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-long".into(), approval_id: Some("call-long".into()), turn_id: "turn-long".into(), diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index 662bd76bab..4b8f1b5258 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -430,6 +430,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { // Trigger an exec approval request with a short, single-line command let ev = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-short".into(), approval_id: Some("call-short".into()), turn_id: "turn-short".into(), @@ -522,6 +523,7 @@ async fn exec_approval_uses_approval_id_when_present() { &mut chat, "sub-short", ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-parent".into(), approval_id: Some("approval-subcommand".into()), turn_id: "turn-short".into(), @@ -566,6 +568,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { // Multiline command: modal should show full command, history records decision only let ev_multi = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-multi".into(), approval_id: Some("call-multi".into()), turn_id: "turn-multi".into(), @@ -620,6 +623,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { // Very long single-line command: decision snippet should be truncated <= 80 chars with trailing ... let long = format!("echo {}", "a".repeat(200)); let ev_long = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-long".into(), approval_id: Some("call-long".into()), turn_id: "turn-long".into(), @@ -1552,6 +1556,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { .set(AskForApproval::OnRequest.to_core())?; // Inject an exec approval request to display the approval modal. let ev = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-approve-cmd".into(), approval_id: Some("call-approve-cmd".into()), turn_id: "turn-approve-cmd".into(), @@ -1610,6 +1615,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { .set(AskForApproval::OnRequest.to_core())?; let ev = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-approve-cmd-noreason".into(), approval_id: Some("call-approve-cmd-noreason".into()), turn_id: "turn-approve-cmd-noreason".into(), @@ -1656,6 +1662,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() let script = "python - <<'PY'\nprint('hello')\nPY".to_string(); let command = vec!["bash".into(), "-lc".into(), script]; let ev = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-approve-cmd-multiline-trunc".into(), approval_id: Some("call-approve-cmd-multiline-trunc".into()), turn_id: "turn-approve-cmd-multiline-trunc".into(), diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__approval_requests__write_stdin_approval_modal.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__approval_requests__write_stdin_approval_modal.snap new file mode 100644 index 0000000000..e0108c8d80 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__approval_requests__write_stdin_approval_modal.snap @@ -0,0 +1,30 @@ +--- +source: tui/src/chatwidget/tests/approval_requests.rs +assertion_line: 123 +expression: "format!(\"{buf:?}\")" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 80, height: 10 }, + content: [ + " ", + " ", + " Would you like to send input to terminal 42? ", + " ", + " Input: "confirm\n" ", + " ", + "› 1. Yes, proceed (y) ", + " 2. No, and tell Codex what to do differently (esc) ", + " ", + " Press enter to confirm or esc to cancel ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 46, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 6, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 21, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 48, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 51, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + ] +} diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index bb65b85b56..a283433b1f 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -2640,6 +2640,7 @@ async fn status_widget_and_approval_modal_snapshot() { // Now show an approval modal (e.g. exec approval). let ev = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-approve-exec".into(), approval_id: Some("call-approve-exec".into()), turn_id: "turn-approve-exec".into(), diff --git a/codex-rs/tui/src/chatwidget/tests/terminal_title.rs b/codex-rs/tui/src/chatwidget/tests/terminal_title.rs index 8a7a438a6c..0d7d089001 100644 --- a/codex-rs/tui/src/chatwidget/tests/terminal_title.rs +++ b/codex-rs/tui/src/chatwidget/tests/terminal_title.rs @@ -81,6 +81,7 @@ async fn terminal_title_shows_action_required_while_exec_approval_is_pending() { ); let request = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-action-required".into(), approval_id: Some("call-action-required".into()), turn_id: "turn-action-required".into(), @@ -146,6 +147,7 @@ async fn terminal_title_action_required_respects_spinner_setting() { chat.refresh_terminal_title(); let request = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-no-spinner".into(), approval_id: Some("call-no-spinner".into()), turn_id: "turn-no-spinner".into(), @@ -174,6 +176,7 @@ async fn terminal_title_action_required_blinks_when_animations_are_enabled() { chat.refresh_terminal_title(); let request = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-blink".into(), approval_id: Some("call-blink".into()), turn_id: "turn-blink".into(), @@ -212,6 +215,7 @@ async fn terminal_title_activity_indicators_do_not_animate_when_animations_are_d assert!(!chat.should_animate_terminal_title_spinner()); let request = ExecApprovalRequestEvent { + kind: Default::default(), call_id: "call-no-animations".into(), approval_id: Some("call-no-animations".into()), turn_id: "turn-no-animations".into(), diff --git a/codex-rs/tui/src/chatwidget/tool_requests.rs b/codex-rs/tui/src/chatwidget/tool_requests.rs index eb549a4154..cf01c394ff 100644 --- a/codex-rs/tui/src/chatwidget/tool_requests.rs +++ b/codex-rs/tui/src/chatwidget/tool_requests.rs @@ -289,6 +289,7 @@ impl ChatWidget { let available_decisions = ev.effective_available_decisions(); let request = ApprovalRequest::Exec(ExecApprovalRequest { + kind: ev.kind, thread_id: self.thread_id.unwrap_or_default(), thread_label: None, id: ev.effective_approval_id(),