diff --git a/codex-rs/app-server/tests/suite/v2/curated_mcp_sync.rs b/codex-rs/app-server/tests/suite/v2/curated_mcp_sync.rs index 957c2645b9..06c464ee34 100644 --- a/codex-rs/app-server/tests/suite/v2/curated_mcp_sync.rs +++ b/codex-rs/app-server/tests/suite/v2/curated_mcp_sync.rs @@ -47,7 +47,8 @@ struct CuratedMcpSyncFixture { impl CuratedMcpSyncFixture { async fn set_up() -> Result { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = + start_mcp_server(/*sensitive_action*/ None).await?; let fixture_root = TempDir::new()?; let codex_home = fixture_root.path().join("codex-home"); let curated_repo = fixture_root.path().join("plugins.git"); diff --git a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs index 558acbf1b6..8cb6a2bb89 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_v2.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_v2.rs @@ -508,6 +508,7 @@ async fn guardian_v2_routes_tool_approvals( review_outcome, transcript_content, GuardianToolScope::AllTools, + /*sensitive_action*/ None, ) .await } @@ -519,6 +520,7 @@ async fn guardian_v2_routes_scoped_tool_approvals( review_outcome: ReviewOutcome, transcript_content: TranscriptContent, scope: GuardianToolScope, + sensitive_action: Option, ) -> Result<()> { let server_name = match scope { GuardianToolScope::AllTools => TEST_SERVER_NAME, @@ -533,7 +535,7 @@ async fn guardian_v2_routes_scoped_tool_approvals( let node_repl_review_required = matches!(requirement, ModelReviewRequirement::Required) && codex_protocol::mcp::is_node_repl_backed_server(server_name); let (luna_score, expected_guardian_reviews) = match risk { - GuardianRisk::Low if classifier_in_scope => (0.25, 1), + GuardianRisk::Low if classifier_in_scope && sensitive_action != Some(true) => (0.25, 1), GuardianRisk::Low | GuardianRisk::InvalidResponse => (0.25, 2), GuardianRisk::Threshold => (0.5, 2), GuardianRisk::High => (0.95, 2), @@ -579,7 +581,7 @@ async fn guardian_v2_routes_scoped_tool_approvals( let responses_server = tokio::spawn(async move { let _ = axum::serve(listener, router).await; }); - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(sensitive_action).await?; let codex_home = TempDir::new()?; if lifecycle.has_post_tool_hook() { @@ -1258,7 +1260,7 @@ async fn guardian_v2_trusts_invoked_user_skills_but_rejects_repository_forgery() let responses_server = tokio::spawn(async move { let _ = axum::serve(listener, router).await; }); - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; MockResponsesConfig::new(&responses_url) .with_model(MODEL) @@ -1393,21 +1395,27 @@ async fn guardian_v2_computer_use_only_scopes_classification_and_fast_reviews( ReviewOutcome::Allow, TranscriptContent::Normal, GuardianToolScope::ComputerUseOnly { server_name }, + /*sensitive_action*/ None, ) .await } -#[test_case("node_repl", GuardianRisk::Low; "browser low risk")] -#[test_case("cua_repl", GuardianRisk::Low; "computer use low risk")] -#[test_case("node_repl", GuardianRisk::High; "browser high risk")] -#[test_case("cua_repl", GuardianRisk::High; "computer use high risk")] -#[test_case("node_repl", GuardianRisk::InvalidResponse; "browser classifier failure")] -#[test_case("cua_repl", GuardianRisk::InvalidResponse; "computer use classifier failure")] -#[test_case(TEST_SERVER_NAME, GuardianRisk::Low; "other tools retain full review")] +#[test_case("node_repl", GuardianRisk::Low, None; "browser low risk")] +#[test_case("cua_repl", GuardianRisk::Low, None; "computer use low risk")] +#[test_case("node_repl", GuardianRisk::Low, Some(false); "browser low risk sensitive action false")] +#[test_case("cua_repl", GuardianRisk::Low, Some(false); "computer use low risk sensitive action false")] +#[test_case("node_repl", GuardianRisk::Low, Some(true); "browser low risk sensitive action true")] +#[test_case("cua_repl", GuardianRisk::Low, Some(true); "computer use low risk sensitive action true")] +#[test_case("node_repl", GuardianRisk::High, None; "browser high risk")] +#[test_case("cua_repl", GuardianRisk::High, None; "computer use high risk")] +#[test_case("node_repl", GuardianRisk::InvalidResponse, None; "browser classifier failure")] +#[test_case("cua_repl", GuardianRisk::InvalidResponse, None; "computer use classifier failure")] +#[test_case(TEST_SERVER_NAME, GuardianRisk::Low, None; "other tools retain full review")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn guardian_v2_required_model_computer_use_preserves_strict_approval( server_name: &'static str, risk: GuardianRisk, + sensitive_action: Option, ) -> Result<()> { skip_if_no_network!(Ok(())); guardian_v2_routes_scoped_tool_approvals( @@ -1417,6 +1425,7 @@ async fn guardian_v2_required_model_computer_use_preserves_strict_approval( ReviewOutcome::Allow, TranscriptContent::Normal, GuardianToolScope::ComputerUseOnly { server_name }, + sensitive_action, ) .await } @@ -1437,6 +1446,7 @@ async fn guardian_v2_discards_sync_reviews_after_user_input_answer( GuardianToolScope::ComputerUseOnly { server_name: "node_repl", }, + /*sensitive_action*/ None, ) .await } @@ -1458,6 +1468,7 @@ async fn guardian_v2_validates_user_input_before_history_truncation( GuardianToolScope::ComputerUseOnly { server_name: "node_repl", }, + /*sensitive_action*/ None, ) .await } @@ -1478,6 +1489,7 @@ async fn guardian_v2_propagates_root_user_input_to_worker_reviews( GuardianToolScope::ComputerUseOnly { server_name: "node_repl", }, + /*sensitive_action*/ None, ) .await } diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 6aa309c4f6..49b886e1e5 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -100,7 +100,7 @@ const LATE_ENVIRONMENT_ID: &str = "late-environment"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -166,7 +166,7 @@ async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_forwards_only_server_extensions() -> Result<()> { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -261,7 +261,7 @@ async fn model_mcp_tool_call_uses_session_client_extensions() -> Result<()> { create_final_assistant_message_sse_response("done")?, ]; let responses_server = create_mock_responses_server_sequence(responses).await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -491,7 +491,7 @@ async fn assert_full_access_form_elicitation_is_declined( case: FullAccessElicitationCase, ) -> Result<()> { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -565,7 +565,7 @@ async fn mcp_server_tool_call_round_trips_elicitation_for_thread( mut elicitation_thread: ElicitationThread, ) -> Result<()> { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -727,7 +727,7 @@ async fn initialize_elicitation_app_server( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_elicitation_survives_environment_runtime_refresh() -> Result<()> { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let exec_listener = TcpListener::bind("127.0.0.1:0").await?; let exec_server_url = format!("ws://{}", exec_listener.local_addr()?); let codex_home = TempDir::new()?; @@ -847,7 +847,7 @@ async fn mcp_server_elicitation_survives_environment_runtime_refresh() -> Result #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_forwards_url_elicitation() -> Result<()> { let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -944,7 +944,7 @@ async fn mcp_tool_call_completion_notification_contains_truncated_large_result() create_final_assistant_message_sse_response("done")?, ]; let responses_server = create_mock_responses_server_sequence(responses).await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config( &responses_server.uri(), @@ -1057,7 +1057,7 @@ async fn mcp_tool_call_hint_survives_mid_call_thread_read_and_resume() -> Result create_final_assistant_message_sse_response("done")?, ]; let responses_server = create_mock_responses_server_sequence(responses).await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; mcp_tool_config(&responses_server.uri(), &mcp_server_url, AUTO_COMPACT_LIMIT) .write(codex_home.path())?; @@ -1196,7 +1196,9 @@ async fn mcp_tool_call_hint_survives_mid_call_thread_read_and_resume() -> Result } #[derive(Clone, Default)] -struct ToolAppsMcpServer; +struct ToolAppsMcpServer { + sensitive_action: Option, +} impl ServerHandler for ToolAppsMcpServer { async fn initialize( @@ -1273,7 +1275,7 @@ impl ServerHandler for ToolAppsMcpServer { .and_then(serde_json::Value::as_bool) == Some(true) { - let approval_meta = json!({ + let mut approval_meta = json!({ "codex_request_type": "approval_request", "codex_approval_kind": "mcp_tool_call", "codex_strict_auto_review": true, @@ -1281,6 +1283,9 @@ impl ServerHandler for ToolAppsMcpServer { "tool_params": request.arguments, "x-codex-turn-metadata": turn_metadata, }); + if let Some(sensitive_action) = self.sensitive_action { + approval_meta["codex_sensitive_action"] = json!(sensitive_action); + } let result = context .peer .create_elicitation(ElicitRequestParams::FormElicitationParams { @@ -1398,11 +1403,13 @@ impl ServerHandler for ToolAppsMcpServer { } } -pub(super) async fn start_mcp_server() -> Result<(String, JoinHandle<()>)> { +pub(super) async fn start_mcp_server( + sensitive_action: Option, +) -> Result<(String, JoinHandle<()>)> { let listener = TcpListener::bind("127.0.0.1:0").await?; let addr = listener.local_addr()?; let mcp_service = StreamableHttpService::new( - || Ok(ToolAppsMcpServer), + move || Ok(ToolAppsMcpServer { sensitive_action }), Arc::new(LocalSessionManager::default()), StreamableHttpServerConfig::default(), ); diff --git a/codex-rs/app-server/tests/suite/v2/view_image.rs b/codex-rs/app-server/tests/suite/v2/view_image.rs index b83a8ed3a1..2d88d82c61 100644 --- a/codex-rs/app-server/tests/suite/v2/view_image.rs +++ b/codex-rs/app-server/tests/suite/v2/view_image.rs @@ -41,7 +41,7 @@ async fn fresh_context_subagent_inherits_disabled_view_image_and_mcp_tools() -> const MCP_CALL_ID: &str = "call-child-mcp-with-disabled-view-image"; let responses_server = responses::start_mock_server().await; - let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let (mcp_server_url, mcp_server_handle) = start_mcp_server(/*sensitive_action*/ None).await?; let codex_home = TempDir::new()?; MockResponsesConfig::new(&responses_server.uri()) .with_model("gpt-5.4") diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index 6aaeffda7a..cd6c2ccfec 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -322,7 +322,8 @@ async fn run_guardian_review( options: GuardianReviewOptions, ) -> ReviewDecision { let turn = Arc::clone(context.turn()); - let requires_synchronous_review = reasons.retry.is_some() + let requires_synchronous_review = options.require_synchronous_review + || reasons.retry.is_some() || matches!( &request, GuardianApprovalRequest::ExecCommand { @@ -331,7 +332,8 @@ async fn run_guardian_review( } if sandbox_permissions.requires_escalated_permissions() ); // Guardian V2 may satisfy ordinary reviews, including required-model reviews, but broader - // permission requests and retries must run Guardian synchronously. + // permission requests, retries, and elicitations requiring synchronous review must not use + // extension fast approval. if (!turn .config .config_layer_stack @@ -368,6 +370,7 @@ async fn run_guardian_review( plugin_attribution_override, approval_request_source, external_cancel, + require_synchronous_review: _, } = options; let target_item_id = guardian_request_target_item_id(&request).map(str::to_string); let assessment_turn_id = guardian_request_turn_id(&request, &turn.sub_id).to_string(); @@ -739,6 +742,8 @@ pub(crate) struct GuardianReviewOptions { pub(crate) plugin_attribution_override: Option, pub(crate) approval_request_source: GuardianApprovalRequestSource, pub(crate) external_cancel: Option, + /// Escalate from extension fast approval to the synchronous Guardian reviewer. + pub(crate) require_synchronous_review: bool, } /// Public entrypoint for approval requests that should be reviewed by guardian. @@ -760,6 +765,7 @@ pub(crate) async fn review_approval_request( plugin_attribution_override: None, approval_request_source: GuardianApprovalRequestSource::MainTurn, external_cancel: None, + require_synchronous_review: false, }, )); review.await diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index f5694761c3..b4faee1ac1 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -1506,6 +1506,7 @@ async fn cancelled_guardian_review_emits_terminal_abort_without_warning() { plugin_attribution_override: None, approval_request_source: GuardianApprovalRequestSource::MainTurn, external_cancel: Some(cancel_token), + require_synchronous_review: false, }, ) .await; diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 026330b541..c0a1d0d64a 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -23,6 +23,7 @@ use codex_protocol::mcp_approval_meta::CONNECTOR_NAME_KEY as MCP_ELICITATION_CON use codex_protocol::mcp_approval_meta::PERSIST_KEY as MCP_ELICITATION_PERSIST_KEY; use codex_protocol::mcp_approval_meta::REQUEST_TYPE_APPROVAL_REQUEST as MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST; use codex_protocol::mcp_approval_meta::REQUEST_TYPE_KEY as MCP_ELICITATION_REQUEST_TYPE_KEY; +use codex_protocol::mcp_approval_meta::SENSITIVE_ACTION_KEY as MCP_ELICITATION_SENSITIVE_ACTION_KEY; use codex_protocol::mcp_approval_meta::STRICT_AUTO_REVIEW_KEY as MCP_ELICITATION_STRICT_AUTO_REVIEW_KEY; use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_ELICITATION_TOOL_DESCRIPTION_KEY; use codex_protocol::mcp_approval_meta::TOOL_NAME_KEY as MCP_ELICITATION_TOOL_NAME_KEY; @@ -723,6 +724,14 @@ async fn review_guardian_mcp_elicitation( return Ok(None); }; + let require_synchronous_review = matches!( + request + .elicitation + .meta() + .and_then(|meta| meta.get(MCP_ELICITATION_SENSITIVE_ACTION_KEY)), + Some(Value::Bool(true)) + ); + if matches!( request .elicitation @@ -809,6 +818,7 @@ async fn review_guardian_mcp_elicitation( plugin_attribution_override: None, approval_request_source: codex_analytics::GuardianApprovalRequestSource::MainTurn, external_cancel: Some(cancellation_token), + require_synchronous_review, }, ) .await; @@ -897,6 +907,7 @@ async fn review_guardian_mcp_elicitation( plugin_attribution_override: None, approval_request_source: codex_analytics::GuardianApprovalRequestSource::MainTurn, external_cancel: Some(cancellation_token), + require_synchronous_review, }, ) .await; diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 8fb75d7360..81a7cfced3 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -631,6 +631,7 @@ impl Session { plugin_attribution_override: None, approval_request_source: GuardianApprovalRequestSource::MainTurn, external_cancel: Some(cancellation_token.clone()), + require_synchronous_review: false, }, ); review.await.unwrap_or_else(|_| { @@ -653,6 +654,7 @@ impl Session { plugin_attribution_override: None, approval_request_source: GuardianApprovalRequestSource::MainTurn, external_cancel: Some(review_cancel), + require_synchronous_review: false, }, ) .await diff --git a/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs b/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs index 77c631e2d5..5ab710bd59 100644 --- a/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs +++ b/codex-rs/core/tests/suite/guardian_mcp_elicitation.rs @@ -3,10 +3,12 @@ use anyhow::Result; use codex_core::TurnInputRequest; use codex_core::config::Constrained; use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::openai_models::AutoReviewMessages; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::GuardianAssessmentStatus; use codex_protocol::protocol::Op; +use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::ThreadSettingsOverrides; use codex_protocol::user_input::UserInput; use core_test_support::responses; @@ -15,9 +17,13 @@ use core_test_support::skip_if_wine_exec; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_mcp_server; +use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; +use std::sync::Arc; use std::time::Duration; +use test_case::test_case; +use wiremock::matchers::body_partial_json; // The tool itself needs no approval. Its server requests a separate Guardian // review after tools/call, exercising the ordinary MCP elicitation path. @@ -29,6 +35,7 @@ def send(message): print(json.dumps({"jsonrpc": "2.0", **message}), flush=True) pending_call = None +approval_meta = json.loads(sys.argv[1]) if len(sys.argv) > 1 else {} for line in sys.stdin: request = json.loads(line) method = request.get("method") @@ -46,7 +53,8 @@ for line in sys.stdin: "message": "Approve the server-side action?", "requestedSchema": {"type": "object", "properties": {}}, "_meta": {"codex_request_type": "approval_request", - "codex_approval_kind": "mcp_tool_call", "tool_name": "write_record"}}}) + "codex_approval_kind": "mcp_tool_call", "tool_name": "write_record", + **approval_meta}}}) continue elif method is None and request.get("id") == "server-approval": send({"id": pending_call, "result": {"content": [ @@ -172,3 +180,164 @@ async fn interrupt_aborts_server_initiated_mcp_guardian_review() -> Result<()> { test.codex.shutdown_and_wait().await?; Ok(()) } + +#[test_case(None, false; "ordinary_absent")] +#[test_case(Some(false), false; "ordinary_false")] +#[test_case(Some(true), false; "ordinary_true")] +#[test_case(None, true; "strict_absent")] +#[test_case(Some(false), true; "strict_false")] +#[test_case(Some(true), true; "strict_true")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn server_initiated_mcp_elicitation_can_require_synchronous_auto_review( + sensitive_action: Option, + strict_auto_review: bool, +) -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_wine_exec!(Ok(()), "the MCP fixture requires a host Python interpreter"); + + const RATIONALE: &str = "The server-side action is not authorized."; + const REJECTION_INSTRUCTIONS: &str = "Do not retry the rejected action."; + + struct AutoApprovingReviewContributor; + + impl codex_extension_api::ApprovalReviewContributor for AutoApprovingReviewContributor { + fn fast_decision<'a>( + &'a self, + _session_store: &'a codex_extension_api::ExtensionData, + _thread_store: &'a codex_extension_api::ExtensionData, + _prompt: &'a str, + _extension_metrics: Option>, + ) -> codex_extension_api::ExtensionFuture<'a, Option> { + Box::pin(async { Some(ReviewDecision::Approved) }) + } + } + + let mut extensions = codex_extension_api::ExtensionRegistryBuilder::new(); + extensions.approval_review_contributor(Arc::new(AutoApprovingReviewContributor)); + let mut meta = serde_json::Map::new(); + if strict_auto_review { + meta.insert("codex_strict_auto_review".to_string(), json!(true)); + } + if let Some(sensitive_action) = sensitive_action { + meta.insert( + "codex_sensitive_action".to_string(), + json!(sensitive_action), + ); + } + + let server = responses::start_mock_server().await; + let mcp_servers = serde_json::from_value(json!({ + "elicitation": { + "command": if cfg!(windows) { "python" } else { "python3" }, + "args": ["-u", "-c", ELICITATION_SERVER, serde_json::to_string(&meta)?], + "default_tools_approval_mode": "approve", + } + }))?; + let mut builder = test_codex() + .with_extensions(Arc::new(extensions.build())) + .with_model_info_override("gpt-5.5", |model| { + model + .model_messages + .as_mut() + .expect("acting model messages") + .auto_review = Some(AutoReviewMessages { + policy: None, + policy_template: None, + rejection_instructions: Some(REJECTION_INSTRUCTIONS.to_string()), + timeout_instructions: None, + }); + }) + .with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config.approvals_reviewer = if strict_auto_review { + ApprovalsReviewer::User + } else { + ApprovalsReviewer::AutoReview + }; + config + .mcp_servers + .set(mcp_servers) + .expect("set MCP fixture"); + }); + let test = builder.build_with_auto_env(&server).await?; + wait_for_mcp_server(&test.codex, "elicitation").await?; + responses::mount_sse_once( + &server, + responses::sse(vec![ + responses::ev_function_call_with_namespace( + "eliciting-tool", + "mcp__elicitation", + "request_approval", + "{}", + ), + responses::ev_completed("parent-tool"), + ]), + ) + .await; + let guardian = responses::mount_sse_once_match( + &server, + body_partial_json(json!({"client_metadata": {"x-openai-subagent": "guardian"}})), + responses::sse(vec![ + responses::ev_assistant_message( + "review-result", + &json!({ + "risk_level": "high", + "user_authorization": "low", + "outcome": "deny", + "rationale": RATIONALE, + }) + .to_string(), + ), + responses::ev_completed("guardian-review"), + ]), + ) + .await; + let parent = responses::mount_sse_once( + &server, + responses::sse(vec![responses::ev_completed("parent-complete")]), + ) + .await; + + test.submit_text_turn("Run the tool that requests server-side approval.") + .await?; + + let output = parent + .single_request() + .function_call_output("eliciting-tool"); + let response: Value = serde_json::from_str( + output["output"][1]["text"] + .as_str() + .expect("MCP fixture should echo the wire response"), + )?; + let requires_sync = sensitive_action == Some(true); + let expected_response = if requires_sync { + json!({ + "action": "decline", + "_meta": { + "approvals_reviewer": "auto_review", + "message": format!( + "This action was rejected due to unacceptable risk.\nReason: {RATIONALE}\n{REJECTION_INSTRUCTIONS}" + ), + }, + }) + } else { + json!({ + "action": "accept", + "content": {}, + "_meta": {"approvals_reviewer": "auto_review"}, + }) + }; + assert_eq!(response, expected_response); + + let guardian_requests = guardian + .requests() + .into_iter() + .filter(|request| request.body_json()["client_metadata"]["x-openai-subagent"] == "guardian") + .collect::>(); + assert_eq!(guardian_requests.len(), usize::from(requires_sync)); + if requires_sync { + assert!(guardian_requests[0].body_contains_text("write_record")); + } + test.codex.shutdown_and_wait().await?; + Ok(()) +} diff --git a/codex-rs/protocol/src/mcp_approval_meta.rs b/codex-rs/protocol/src/mcp_approval_meta.rs index 4aebdd9e67..b5323708d8 100644 --- a/codex-rs/protocol/src/mcp_approval_meta.rs +++ b/codex-rs/protocol/src/mcp_approval_meta.rs @@ -7,6 +7,9 @@ pub const APPROVAL_KIND_TOOL_SUGGESTION: &str = "tool_suggestion"; pub const REQUEST_TYPE_KEY: &str = "codex_request_type"; pub const REQUEST_TYPE_APPROVAL_REQUEST: &str = "approval_request"; pub const STRICT_AUTO_REVIEW_KEY: &str = "codex_strict_auto_review"; +/// Marks a sensitive action requiring synchronous review when routed to automatic review. +/// Absent or false preserves the existing approval path; this does not opt into review. +pub const SENSITIVE_ACTION_KEY: &str = "codex_sensitive_action"; pub const APPROVALS_REVIEWER_KEY: &str = "approvals_reviewer"; pub const PERSIST_KEY: &str = "persist"; pub const PERSIST_SESSION: &str = "session";