diff --git a/codex-rs/analytics/src/reducer.rs b/codex-rs/analytics/src/reducer.rs index d0707e36c3..26cd458bd3 100644 --- a/codex-rs/analytics/src/reducer.rs +++ b/codex-rs/analytics/src/reducer.rs @@ -1743,7 +1743,8 @@ impl AnalyticsReducer { ServerRequest::CommandExecutionRequestApproval { request_id, params } => { let is_stdin_review = match params.kind { CommandExecutionApprovalKind::WriteStdin => true, - CommandExecutionApprovalKind::Command => false, + CommandExecutionApprovalKind::Command + | CommandExecutionApprovalKind::HumanCommand => false, }; let is_network_access_review = params.network_approval_context.is_some(); let requested_network_access = is_network_access_review @@ -1760,7 +1761,9 @@ impl AnalyticsReducer { let requested_additional_permissions = params.additional_permissions.is_some(); let trigger = if is_stdin_review { ReviewTrigger::Initial - } else if params.approval_id.is_some() { + } else if params.kind != CommandExecutionApprovalKind::HumanCommand + && params.approval_id.is_some() + { ReviewTrigger::ExecveIntercept } else if requested_network_access { ReviewTrigger::NetworkPolicyDenial diff --git a/codex-rs/analytics/src/tests/suite/approvals_tests.rs b/codex-rs/analytics/src/tests/suite/approvals_tests.rs index 03caca5d5c..c49a7570a5 100644 --- a/codex-rs/analytics/src/tests/suite/approvals_tests.rs +++ b/codex-rs/analytics/src/tests/suite/approvals_tests.rs @@ -258,6 +258,7 @@ async fn guardian_review_event_ingests_custom_fact_with_optional_target_item() { version: "1.0.0".to_string(), }, capabilities: Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, diff --git a/codex-rs/analytics/src/tests/suite/threads_tests.rs b/codex-rs/analytics/src/tests/suite/threads_tests.rs index b0c4cc0f08..9c0dd2dd79 100644 --- a/codex-rs/analytics/src/tests/suite/threads_tests.rs +++ b/codex-rs/analytics/src/tests/suite/threads_tests.rs @@ -217,6 +217,7 @@ async fn initialize_caches_client_and_thread_lifecycle_publishes_once_initialize version: "1.0.0".to_string(), }, capabilities: Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, diff --git a/codex-rs/analytics/src/tests/suite/turns/compaction_tests.rs b/codex-rs/analytics/src/tests/suite/turns/compaction_tests.rs index 2c0400af4b..88f86a39f2 100644 --- a/codex-rs/analytics/src/tests/suite/turns/compaction_tests.rs +++ b/codex-rs/analytics/src/tests/suite/turns/compaction_tests.rs @@ -145,6 +145,7 @@ async fn compaction_events_keep_authoritative_voice_session_across_consecutive_c version: "1.0.0".to_string(), }, capabilities: Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, diff --git a/codex-rs/analytics/src/tests/support.rs b/codex-rs/analytics/src/tests/support.rs index c47593d50d..ba1df28286 100644 --- a/codex-rs/analytics/src/tests/support.rs +++ b/codex-rs/analytics/src/tests/support.rs @@ -592,6 +592,7 @@ pub(super) fn sample_initialize_fact(connection_id: u64) -> AnalyticsFact { version: "1.0.0".to_string(), }, capabilities: Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index e7df794ea8..c4b9011f60 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -207,6 +207,8 @@ where #[derive(Clone)] pub struct InProcessClientStartArgs { + /// This client collects fresh human decisions for managed command checkpoints. + pub required_human_approval: bool, /// Resolved argv0 dispatch paths used by command execution internals. pub arg0_paths: Arg0DispatchPaths, /// Shared config used to initialize app-server runtime. @@ -253,6 +255,7 @@ impl InProcessClientStartArgs { /// Builds initialize params from caller-provided metadata. pub fn initialize_params(&self) -> InitializeParams { let capabilities = InitializeCapabilities { + required_human_approval: self.required_human_approval, explicit_gateway_oauth: false, experimental_api: self.experimental_api, request_attestation: false, @@ -949,6 +952,7 @@ mod tests { .await .expect("state db should initialize for in-process test"); let client = InProcessAppServerClient::start(InProcessClientStartArgs { + required_human_approval: false, arg0_paths: Arg0DispatchPaths::default(), config, cli_overrides: Vec::new(), @@ -1172,6 +1176,7 @@ mod tests { fn test_remote_connect_args(websocket_url: String) -> RemoteAppServerConnectArgs { RemoteAppServerConnectArgs { + required_human_approval: false, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url, auth_token: None, @@ -1536,6 +1541,7 @@ mod tests { websocket.close(None).await.expect("close should succeed"); }); let client = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: false, endpoint: RemoteAppServerEndpoint::UnixSocket { socket_path }, client_name: "codex-app-server-client-test".to_string(), client_version: "0.0.0-test".to_string(), @@ -1623,6 +1629,7 @@ mod tests { ) .await; let client = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: false, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url, auth_token: Some(auth_token), @@ -1643,6 +1650,7 @@ mod tests { #[tokio::test] async fn remote_connect_rejects_non_loopback_ws_when_auth_configured() { let result = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: false, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url: "ws://example.com:4500".to_string(), auth_token: Some("remote-bearer-token".to_string()), @@ -2178,6 +2186,7 @@ mod tests { ); let runtime_args = InProcessClientStartArgs { + required_human_approval: false, arg0_paths: Arg0DispatchPaths::default(), config: config.clone(), cli_overrides: Vec::new(), diff --git a/codex-rs/app-server-client/src/remote.rs b/codex-rs/app-server-client/src/remote.rs index ad8d28bcec..c6ac801872 100644 --- a/codex-rs/app-server-client/src/remote.rs +++ b/codex-rs/app-server-client/src/remote.rs @@ -84,6 +84,8 @@ pub enum RemoteAppServerEndpoint { #[derive(Debug, Clone)] pub struct RemoteAppServerConnectArgs { + /// This client collects fresh human decisions for managed command checkpoints. + pub required_human_approval: bool, pub endpoint: RemoteAppServerEndpoint, pub client_name: String, pub client_version: String, @@ -95,6 +97,7 @@ pub struct RemoteAppServerConnectArgs { impl RemoteAppServerConnectArgs { pub(crate) fn initialize_params(&self) -> InitializeParams { let capabilities = InitializeCapabilities { + required_human_approval: self.required_human_approval, explicit_gateway_oauth: false, experimental_api: self.experimental_api, request_attestation: false, diff --git a/codex-rs/app-server-client/tests/remote_compression.rs b/codex-rs/app-server-client/tests/remote_compression.rs index cbab159c1d..dcda4d9705 100644 --- a/codex-rs/app-server-client/tests/remote_compression.rs +++ b/codex-rs/app-server-client/tests/remote_compression.rs @@ -82,6 +82,7 @@ async fn compressed_session_negotiates_fragments_and_retains_history() -> Result }; let client = async { let mut client = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: false, endpoint: RemoteAppServerEndpoint::UnixSocket { socket_path }, client_name: "compression-test".to_string(), client_version: "0.0.0".to_string(), diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index ccb590e3a9..32ecea8835 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1760,6 +1760,10 @@ "default": false, "description": "Opt into `attestation/generate` requests for upstream `x-oai-attestation`.", "type": "boolean" + }, + "requiredHumanApproval": { + "description": "Present `humanCommand` approvals to a person and return only a one-time decision. Requires `experimentalApi`. Pending requests can be replayed to compatible clients.", + "type": "boolean" } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json index 4240a7b972..cb9598d966 100644 --- a/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json +++ b/codex-rs/app-server-protocol/schema/json/CommandExecutionRequestApprovalParams.json @@ -271,11 +271,22 @@ }, "CommandExecutionApprovalKind": { "description": "Distinguishes a command approval from input sent to an existing terminal.", - "enum": [ - "command", - "writeStdin" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "command", + "writeStdin" + ], + "type": "string" + }, + { + "description": "A managed checkpoint requiring a fresh human decision. Only `accept`, `decline`, and `cancel` are valid. Requires the `requiredHumanApproval` initialize capability. This prompt does not itself start a command execution item.", + "enum": [ + "humanCommand" + ], + "type": "string" + } + ] }, "FileSystemAccessMode": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/ServerRequest.json b/codex-rs/app-server-protocol/schema/json/ServerRequest.json index 3b849cabaa..cf9fb5e86b 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ServerRequest.json @@ -340,11 +340,22 @@ }, "CommandExecutionApprovalKind": { "description": "Distinguishes a command approval from input sent to an existing terminal.", - "enum": [ - "command", - "writeStdin" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "command", + "writeStdin" + ], + "type": "string" + }, + { + "description": "A managed checkpoint requiring a fresh human decision. Only `accept`, `decline`, and `cancel` are valid. Requires the `requiredHumanApproval` initialize capability. This prompt does not itself start a command execution item.", + "enum": [ + "humanCommand" + ], + "type": "string" + } + ] }, "CommandExecutionRequestApprovalParams": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 7951770aa0..22b81519fc 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -2840,11 +2840,22 @@ }, "CommandExecutionApprovalKind": { "description": "Distinguishes a command approval from input sent to an existing terminal.", - "enum": [ - "command", - "writeStdin" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "command", + "writeStdin" + ], + "type": "string" + }, + { + "description": "A managed checkpoint requiring a fresh human decision. Only `accept`, `decline`, and `cancel` are valid. Requires the `requiredHumanApproval` initialize capability. This prompt does not itself start a command execution item.", + "enum": [ + "humanCommand" + ], + "type": "string" + } + ] }, "CommandExecutionRequestApprovalParams": { "$schema": "http://json-schema.org/draft-07/schema#", @@ -3437,6 +3448,10 @@ "default": false, "description": "Opt into `attestation/generate` requests for upstream `x-oai-attestation`.", "type": "boolean" + }, + "requiredHumanApproval": { + "description": "Present `humanCommand` approvals to a person and return only a one-time decision. Requires `experimentalApi`. Pending requests can be replayed to compatible clients.", + "type": "boolean" } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 170870f46d..3204a638de 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -10095,6 +10095,10 @@ "default": false, "description": "Opt into `attestation/generate` requests for upstream `x-oai-attestation`.", "type": "boolean" + }, + "requiredHumanApproval": { + "description": "Present `humanCommand` approvals to a person and return only a one-time decision. Requires `experimentalApi`. Pending requests can be replayed to compatible clients.", + "type": "boolean" } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/json/v1/InitializeParams.json b/codex-rs/app-server-protocol/schema/json/v1/InitializeParams.json index 750dddeaf5..2b54970c56 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/InitializeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/InitializeParams.json @@ -60,6 +60,10 @@ "default": false, "description": "Opt into `attestation/generate` requests for upstream `x-oai-attestation`.", "type": "boolean" + }, + "requiredHumanApproval": { + "description": "Present `humanCommand` approvals to a person and return only a one-time decision. Requires `experimentalApi`. Pending requests can be replayed to compatible clients.", + "type": "boolean" } }, "type": "object" diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index 9f0c5f19b4..25ba20b956 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ 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 a4070a59a9..72a89ab3b2 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-protocol/schema/typescript/InitializeCapabilities.ts b/codex-rs/app-server-protocol/schema/typescript/InitializeCapabilities.ts index ac82d743e3..c9f74b309a 100644 --- a/codex-rs/app-server-protocol/schema/typescript/InitializeCapabilities.ts +++ b/codex-rs/app-server-protocol/schema/typescript/InitializeCapabilities.ts @@ -7,6 +7,11 @@ import type { JsonValue } from "./serde_json/JsonValue"; * Client-declared capabilities negotiated during initialize. */ export type InitializeCapabilities = { +/** + * Present `humanCommand` approvals to a person and return only a one-time decision. + * Requires `experimentalApi`. Pending requests can be replayed to compatible clients. + */ +requiredHumanApproval?: boolean, /** * Use explicit gateway OAuth login instead of automatic browser authorization. * Applies to this app-server's gateway runtime; later connections cannot undo it. diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts index f94eac8da4..719cdce04f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/CommandExecutionApprovalKind.ts @@ -5,4 +5,4 @@ /** * Distinguishes a command approval from input sent to an existing terminal. */ -export type CommandExecutionApprovalKind = "command" | "writeStdin"; +export type CommandExecutionApprovalKind = "command" | "humanCommand" | "writeStdin"; diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 2936c01855..5a366bf913 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -2835,6 +2835,7 @@ mod tests { version: "0.1.0".to_string(), }, capabilities: Some(v1::InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: true, @@ -2923,6 +2924,7 @@ mod tests { version: "0.1.0".to_string(), }, capabilities: Some(v1::InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: true, diff --git a/codex-rs/app-server-protocol/src/protocol/v1.rs b/codex-rs/app-server-protocol/src/protocol/v1.rs index 7d05fb1bd4..cdf6ca20a4 100644 --- a/codex-rs/app-server-protocol/src/protocol/v1.rs +++ b/codex-rs/app-server-protocol/src/protocol/v1.rs @@ -44,6 +44,10 @@ pub struct ClientInfo { #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema, TS)] #[serde(rename_all = "camelCase")] pub struct InitializeCapabilities { + /// Present `humanCommand` approvals to a person and return only a one-time decision. + /// Requires `experimentalApi`. Pending requests can be replayed to compatible clients. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub required_human_approval: bool, /// Use explicit gateway OAuth login instead of automatic browser authorization. /// Applies to this app-server's gateway runtime; later connections cannot undo it. #[serde(default, skip_serializing_if = "std::ops::Not::not")] diff --git a/codex-rs/app-server-protocol/src/protocol/v2/item.rs b/codex-rs/app-server-protocol/src/protocol/v2/item.rs index f496ef172b..5ab9f9f0e2 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/item.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/item.rs @@ -1644,6 +1644,10 @@ v2_enum_from_core! { pub enum CommandExecutionApprovalKind from CoreExecApprovalKind { #[default] Command, + /// A managed checkpoint requiring a fresh human decision. Only `accept`, `decline`, and + /// `cancel` are valid. Requires the `requiredHumanApproval` initialize capability. + /// This prompt does not itself start a command execution item. + HumanCommand, WriteStdin, } } diff --git a/codex-rs/app-server-test-client/src/lib.rs b/codex-rs/app-server-test-client/src/lib.rs index ae04387445..63eac5c12f 100644 --- a/codex-rs/app-server-test-client/src/lib.rs +++ b/codex-rs/app-server-test-client/src/lib.rs @@ -1608,6 +1608,7 @@ impl CodexClient { version: env!("CARGO_PKG_VERSION").to_string(), }, capabilities: Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api, request_attestation: false, diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index acb7a9ac21..aade06a992 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1,3 +1,20 @@ +# Managed human command checkpoints (experimental) + +A matching managed `requirements.toml` prefix rule with `decision = "prompt-user"` +requires a fresh human decision, including when the thread uses automatic review. +Prefix matching, exact-path precedence, and executable mappings stay unchanged. Clients +supporting this workflow must initialize with `experimentalApi: true` and +`requiredHumanApproval: true`. + +The server sends `item/commandExecution/requestApproval` with `kind: "humanCommand"`, +a unique `approvalId`, and the command to display. Only `accept`, `decline`, and +`cancel` are valid responses. Session and persistent approvals cannot satisfy the +checkpoint. Only capable clients receive or answer these requests. Pending prompts +use the normal approval replay flow when a capable client reconnects. With no capable +connection when approval is requested, the command is rejected. The prompt itself +does not start an execution item; normal command execution events report execution +after approval. + # Thread memory summary (experimental) `thread/memory/read` takes `{threadId}` for a loaded thread and returns diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index a4130d93fd..9ef4657a36 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -751,7 +751,8 @@ pub(crate) async fn apply_bespoke_event_handling( Some(shlex_join(&command)), Some(completion_item.cwd.clone()), Some(command_actions), - Some(completion_item), + (kind != codex_protocol::approvals::ExecApprovalKind::HumanCommand) + .then_some(completion_item), ), }; if approval_starts_command && let Some(completion_item) = completion_item.as_ref() { diff --git a/codex-rs/app-server/src/message_processor_thread_lifecycle_tests.rs b/codex-rs/app-server/src/message_processor_thread_lifecycle_tests.rs index 752452815d..26fdf46839 100644 --- a/codex-rs/app-server/src/message_processor_thread_lifecycle_tests.rs +++ b/codex-rs/app-server/src/message_processor_thread_lifecycle_tests.rs @@ -1,4 +1,4 @@ -//! Exercises thread cleanup when startup or child creation outlives the client connection. +//! Exercises client attachment, approval routing, and cleanup across thread creation. use codex_app_server_protocol::TurnStartResponse; use codex_features::Feature; @@ -24,8 +24,11 @@ use anyhow::Result; use app_test_support::MockResponsesConfig; use app_test_support::create_mock_responses_server_sequence_unchecked; use codex_app_server_protocol::InitializeResponse; +use codex_app_server_protocol::JSONRPCError; +use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::ThreadClosedNotification; use codex_app_server_protocol::ThreadLoadedListResponse; use codex_app_server_protocol::ThreadStartResponse; @@ -510,6 +513,233 @@ async fn disconnected_v2_child_lifecycle(unload_delay_secs: u64, ephemeral: bool result } +#[tokio::test] +async fn child_human_approval_survives_attachment_and_rejects_unrelated_clients() -> Result<()> { + let (release_spawn_completion, spawn_completion_gate) = oneshot::channel(); + let (server, _completions) = start_streaming_sse_server(vec![ + vec![ + StreamingSseChunk { + gate: None, + body: responses::sse(vec![ + responses::ev_response_created("spawn"), + responses::ev_function_call_with_namespace( + "spawn", + "collaboration", + "spawn_agent", + &json!({"task_name": "child", "message": "Run echo", "fork_turns": "none"}) + .to_string(), + ), + ]), + }, + StreamingSseChunk { + gate: Some(spawn_completion_gate), + body: responses::sse(vec![responses::ev_completed("spawn")]), + }, + ], + vec![StreamingSseChunk { + gate: None, + body: responses::sse(vec![ + responses::ev_response_created("command"), + responses::ev_function_call( + "child-command", + "exec_command", + &json!({"cmd": "echo human-approved"}).to_string(), + ), + responses::ev_completed("command"), + ]), + }], + ]) + .await; + let home = TempDir::new()?; + std::fs::write( + home.path().join("requirements.toml"), + r#" +[features] +unified_exec = false +shell_tool = true + +[[rules.prefix_rules]] +pattern = [{ token = "echo" }] +decision = "prompt-user" +"#, + )?; + MockResponsesConfig::new(server.uri()) + .with_approval_policy("on-request") + .with_sandbox_mode("workspace-write") + .with_root_config("approvals_reviewer = \"auto_review\"") + .enable_feature(Feature::Collab) + .enable_feature(Feature::MultiAgentV2) + .write(home.path())?; + let config = Arc::new( + ConfigBuilder::default() + .codex_home(home.path().to_path_buf()) + .build() + .await?, + ); + let auth = + AuthManager::shared_from_config(config.as_ref(), /*enable_codex_api_key_env*/ false) + .await?; + let (processor, mut outgoing) = build_test_processor(config, auth).await; + let mut created = processor.thread_created_receiver(); + let incompatible_id = ConnectionId(1); + let capable_id = ConnectionId(2); + let unrelated_id = ConnectionId(3); + let incompatible = Arc::new(ConnectionSessionState::new(ConnectionOrigin::WebSocket)); + let capable = Arc::new(ConnectionSessionState::new(ConnectionOrigin::WebSocket)); + let unrelated = Arc::new(ConnectionSessionState::new(ConnectionOrigin::WebSocket)); + let result: Result<()> = async { + for (id, session, required_human_approval) in [ + (incompatible_id, &incompatible, false), + (capable_id, &capable, true), + ] { + send_request( + &processor, + id, + session, + json!({"id": 1, "method": "initialize", "params": { + "clientInfo": {"name": "attachment-test", "version": "1"}, + "capabilities": { + "experimentalApi": true, + "requiredHumanApproval": required_human_approval + } + }}), + ) + .await; + let _: InitializeResponse = + read_response_from(&mut outgoing, id, /*request_id*/ 1).await; + processor + .connection_initialized(id, session.request_attestation()) + .await; + } + send_request( + &processor, + incompatible_id, + &incompatible, + json!({"id": 2, "method": "thread/start", "params": {}}), + ) + .await; + let parent: ThreadStartResponse = + read_response_from(&mut outgoing, incompatible_id, /*request_id*/ 2).await; + send_request( + &processor, + incompatible_id, + &incompatible, + json!({"id": 3, "method": "turn/start", "params": { + "threadId": parent.thread.id, + "input": [{"type": "text", "text": "Spawn a child to run echo"}] + }}), + ) + .await; + let _: TurnStartResponse = + read_response_from(&mut outgoing, incompatible_id, /*request_id*/ 3).await; + let child_id = timeout(Duration::from_secs(/*secs*/ 10), created.recv()).await??; + + while outgoing.try_recv().is_ok() {} + // Attach the initial client set before draining the child's queued approval. + processor + .try_attach_thread_listener(child_id, vec![incompatible_id, capable_id]) + .await; + let request = timeout(Duration::from_secs(/*secs*/ 10), async { + loop { + if let OutgoingEnvelope::ToConnection { + connection_id, + message: OutgoingMessage::Request(request), + .. + } = outgoing.recv().await.context("outgoing channel closed")? + { + assert_eq!(connection_id, capable_id); + break anyhow::Ok(request); + } + } + }) + .await + .context("capable client did not receive the child's approval during attachment")??; + let ServerRequest::CommandExecutionRequestApproval { request_id, params } = &request else { + anyhow::bail!("expected human approval, got {request:?}"); + }; + assert_eq!( + (¶ms.thread_id, params.kind), + ( + &child_id.to_string(), + codex_app_server_protocol::CommandExecutionApprovalKind::HumanCommand, + ), + ); + + // Initialize a capable client after auto-attachment. Capability alone must not + // let it approve or reject this request before it subscribes and receives it. + send_request( + &processor, + unrelated_id, + &unrelated, + json!({"id": 1, "method": "initialize", "params": { + "clientInfo": {"name": "unrelated-client", "version": "1"}, + "capabilities": {"experimentalApi": true, "requiredHumanApproval": true} + }}), + ) + .await; + let _: InitializeResponse = + read_response_from(&mut outgoing, unrelated_id, /*request_id*/ 1).await; + processor + .connection_initialized(unrelated_id, unrelated.request_attestation()) + .await; + for connection_id in [incompatible_id, unrelated_id] { + processor + .process_response( + connection_id, + JSONRPCResponse { + id: request_id.clone(), + result: json!({"decision": "accept"}), + }, + ) + .await; + processor + .process_error( + connection_id, + JSONRPCError { + id: request_id.clone(), + error: crate::error_code::internal_error("unsupported request"), + }, + ) + .await; + assert_eq!( + processor + .outgoing + .pending_requests_for_thread(child_id) + .await, + vec![request.clone()], + "unrouted responses and errors must leave the request pending", + ); + } + + // Routing ends at the human decision. Command execution and replay have + // their own coverage; declining here needs no Linux sandbox executable. + processor + .process_response( + capable_id, + JSONRPCResponse { + id: request_id.clone(), + result: json!({"decision": "decline"}), + }, + ) + .await; + assert!( + processor + .outgoing + .pending_requests_for_thread(child_id) + .await + .is_empty() + ); + Ok(()) + } + .await; + drop(release_spawn_completion); + drop(outgoing); + processor.clear_runtime_references(); + processor.shutdown_threads().await; + server.shutdown().await; + result +} + async fn send_request( processor: &Arc, connection_id: ConnectionId, diff --git a/codex-rs/app-server/src/outgoing_message.rs b/codex-rs/app-server/src/outgoing_message.rs index 35f5027f7c..d07a14830e 100644 --- a/codex-rs/app-server/src/outgoing_message.rs +++ b/codex-rs/app-server/src/outgoing_message.rs @@ -136,6 +136,7 @@ pub(crate) enum OutgoingEnvelope { pub(crate) struct OutgoingMessageSender { verification_auth: OnceLock>, verification_connections: Mutex>, + human_approval_connections: Mutex>, next_server_request_id: AtomicI64, sender: mpsc::Sender, process_reply_sender: mpsc::Sender, @@ -155,6 +156,7 @@ pub(crate) struct ThreadScopedOutgoingMessageSender { } struct PendingCallbackEntry { + human_approval_recipients: HashSet, verification_owner: Option, verification_auth_revision: Option, verification_identity: Option, @@ -259,6 +261,7 @@ impl OutgoingMessageSender { Self { verification_auth: OnceLock::new(), verification_connections: Mutex::new(HashSet::new()), + human_approval_connections: Mutex::new(HashSet::new()), next_server_request_id: AtomicI64::new(0), process_reply_sender: sender.clone(), sender, @@ -304,7 +307,18 @@ impl OutgoingMessageSender { } } + pub(crate) async fn enable_human_approval_connection(&self, connection_id: ConnectionId) { + self.human_approval_connections + .lock() + .await + .insert(connection_id); + } + pub(crate) async fn connection_closed(&self, connection_id: ConnectionId) { + self.human_approval_connections + .lock() + .await + .remove(&connection_id); self.disconnect_user_verification_connection(connection_id) .await; let mut request_contexts = self.request_contexts.lock().await; @@ -387,6 +401,22 @@ impl OutgoingMessageSender { || verification_identity != self.verification_identity()) }; let (tx_approve, rx_approve) = oneshot::channel(); + let human_connections; + let connection_ids = if requires_human_approval(&request) { + let eligible = self.human_approval_connections.lock().await; + human_connections = connection_ids + .unwrap_or_default() + .iter() + .copied() + .filter(|id| eligible.contains(id)) + .collect::>(); + if human_connections.is_empty() { + return (outgoing_message_id, rx_approve); + } + Some(human_connections.as_slice()) + } else { + connection_ids + }; // One app owns this ceremony. Reconnect and other subscribers cannot answer it. let verification_owner = if user_verification { let eligible = self.verification_connections.lock().await; @@ -409,6 +439,11 @@ impl OutgoingMessageSender { request_id_to_callback.insert( id, PendingCallbackEntry { + human_approval_recipients: if requires_human_approval(&request) { + connection_ids.unwrap_or_default().iter().copied().collect() + } else { + HashSet::new() + }, verification_owner, verification_auth_revision, verification_identity: verification_identity.clone(), @@ -481,7 +516,24 @@ impl OutgoingMessageSender { thread_id: ThreadId, ) { let requests = self.pending_requests_for_thread(thread_id).await; + let human_approval = self + .human_approval_connections + .lock() + .await + .contains(&connection_id); for request in requests { + if requires_human_approval(&request) { + if !human_approval { + continue; + } + // Register the replay recipient before sending so its response + // cannot race ahead of callback authorization. + let mut callbacks = self.request_id_to_callback.lock().await; + let Some(entry) = callbacks.get_mut(request.id()) else { + continue; + }; + entry.human_approval_recipients.insert(connection_id); + } if let Err(err) = self .sender .send(OutgoingEnvelope::ToConnection { @@ -595,8 +647,18 @@ impl OutgoingMessageSender { connection_id: ConnectionId, id: &RequestId, ) -> Option<(RequestId, PendingCallbackEntry)> { + let human_approval = self + .human_approval_connections + .lock() + .await + .contains(&connection_id); let mut callbacks = self.request_id_to_callback.lock().await; let entry = callbacks.get(id)?; + if requires_human_approval(&entry.request) + && (!human_approval || !entry.human_approval_recipients.contains(&connection_id)) + { + return None; + } if let Some(owner) = entry.verification_owner { if owner != connection_id { return None; @@ -952,6 +1014,11 @@ fn timestamped_server_notification(notification: ServerNotification) -> Outgoing }) } +fn requires_human_approval(request: &ServerRequest) -> bool { + matches!(request, ServerRequest::CommandExecutionRequestApproval { params, .. } + if params.kind == codex_app_server_protocol::CommandExecutionApprovalKind::HumanCommand) +} + #[cfg(test)] #[path = "thread_sample_cancel_context_tests.rs"] mod thread_sample_cancel_context_tests; diff --git a/codex-rs/app-server/src/request_processors/initialize_processor.rs b/codex-rs/app-server/src/request_processors/initialize_processor.rs index 6259cf7488..f6b282e7e1 100644 --- a/codex-rs/app-server/src/request_processors/initialize_processor.rs +++ b/codex-rs/app-server/src/request_processors/initialize_processor.rs @@ -141,6 +141,11 @@ impl InitializeRequestProcessor { { return Err(invalid_request("Already initialized")); } + if experimental_api_enabled && capabilities.required_human_approval { + self.outgoing + .enable_human_approval_connection(connection_id) + .await; + } if user_verification_enabled { self.outgoing .enable_user_verification_connection(connection_id) diff --git a/codex-rs/app-server/tests/suite/v2/attestation.rs b/codex-rs/app-server/tests/suite/v2/attestation.rs index 972b4ce1d8..d166e3e3c0 100644 --- a/codex-rs/app-server/tests/suite/v2/attestation.rs +++ b/codex-rs/app-server/tests/suite/v2/attestation.rs @@ -84,6 +84,7 @@ async fn attestation_generate_round_trip_adds_header_to_responses_websocket_hand version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: true, diff --git a/codex-rs/app-server/tests/suite/v2/daemon_update_recovery.rs b/codex-rs/app-server/tests/suite/v2/daemon_update_recovery.rs index b386ace52f..60b7d07cd8 100644 --- a/codex-rs/app-server/tests/suite/v2/daemon_update_recovery.rs +++ b/codex-rs/app-server/tests/suite/v2/daemon_update_recovery.rs @@ -53,6 +53,7 @@ async fn managed_restart_resumes_loaded_threads_and_goal_without_client() -> Res let mut client = connect_daemon_client( &socket_path, InitializeCapabilities { + required_human_approval: false, experimental_api: true, extensions: Some(std::collections::HashMap::from([( "io.modelcontextprotocol/ui".into(), @@ -132,6 +133,7 @@ async fn managed_restart_resumes_loaded_threads_and_goal_without_client() -> Res let mut reconnected = connect_daemon_client( &socket_path, InitializeCapabilities { + required_human_approval: false, experimental_api: true, extensions: Some(std::collections::HashMap::from([( "io.modelcontextprotocol/ui".into(), diff --git a/codex-rs/app-server/tests/suite/v2/experimental_api.rs b/codex-rs/app-server/tests/suite/v2/experimental_api.rs index 39d0d9b65f..c7fc4be42f 100644 --- a/codex-rs/app-server/tests/suite/v2/experimental_api.rs +++ b/codex-rs/app-server/tests/suite/v2/experimental_api.rs @@ -41,6 +41,7 @@ async fn mock_experimental_method_requires_experimental_api_capability() -> Resu .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -117,6 +118,7 @@ async fn realtime_conversation_start_requires_experimental_api_capability() -> R .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -177,6 +179,7 @@ async fn thread_memory_mode_set_requires_experimental_api_capability() -> Result .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -220,6 +223,7 @@ async fn settings_update_requires_experimental_api_capability(method: &str) -> R .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -260,6 +264,7 @@ async fn realtime_webrtc_start_requires_experimental_api_capability() -> Result< .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -323,6 +328,7 @@ async fn thread_start_mock_field_requires_experimental_api_capability() -> Resul .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -367,6 +373,7 @@ async fn thread_start_without_dynamic_tools_allows_without_experimental_api_capa .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, @@ -410,6 +417,7 @@ async fn thread_start_granular_approval_policy_requires_experimental_api_capabil .initialize_with_capabilities( default_client_info(), Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: false, request_attestation: false, diff --git a/codex-rs/app-server/tests/suite/v2/initialize.rs b/codex-rs/app-server/tests/suite/v2/initialize.rs index 1ba0f266c9..2d181de648 100644 --- a/codex-rs/app-server/tests/suite/v2/initialize.rs +++ b/codex-rs/app-server/tests/suite/v2/initialize.rs @@ -240,6 +240,7 @@ async fn initialize_opt_out_notification_methods_filters_notifications() -> Resu version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: false, diff --git a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs index cda1f49377..b6cac451d5 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs @@ -521,6 +521,7 @@ async fn initialize_websocket_client( version: "0.1.0".to_string(), }, capabilities: Some(InitializeCapabilities { + required_human_approval: false, experimental_api: true, extensions: supports_openai_form_elicitation .then(|| HashMap::from([("openai/form".to_string(), serde_json::json!({}))])), 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 5820ab23c8..ca2e06e467 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -260,6 +260,7 @@ async fn mcp_server_tool_call_forwards_only_server_extensions() -> Result<()> { version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: false, @@ -356,6 +357,7 @@ async fn model_mcp_tool_call_uses_session_client_extensions() -> Result<()> { version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: false, @@ -783,6 +785,7 @@ async fn initialize_elicitation_app_server( version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, experimental_api: true, extensions: client_advertises_standard_form_input.then(|| { HashMap::from([( diff --git a/codex-rs/app-server/tests/suite/v2/thread_revert.rs b/codex-rs/app-server/tests/suite/v2/thread_revert.rs index ffbdbdef42..8ba0f07f07 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_revert.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_revert.rs @@ -756,6 +756,7 @@ async fn initialize_experimental(mcp: &mut TestAppServer) -> Result<()> { version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: false, diff --git a/codex-rs/app-server/tests/suite/v2/thread_status.rs b/codex-rs/app-server/tests/suite/v2/thread_status.rs index 23031f6d9e..213ace5936 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_status.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_status.rs @@ -148,6 +148,7 @@ async fn thread_status_changed_can_be_opted_out() -> Result<()> { version: "0.1.0".to_string(), }, Some(InitializeCapabilities { + required_human_approval: false, explicit_gateway_oauth: false, experimental_api: true, request_attestation: false, diff --git a/codex-rs/cli/tests/queue.rs b/codex-rs/cli/tests/queue.rs index eff0d90da9..8797f84c26 100644 --- a/codex-rs/cli/tests/queue.rs +++ b/codex-rs/cli/tests/queue.rs @@ -335,6 +335,7 @@ async fn remote_session_commands_with_workload_identity_use_server_auth() -> Res let endpoint = format!("unix://{}", socket_path.as_path().display()); let mut app = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: false, endpoint: RemoteAppServerEndpoint::UnixSocket { socket_path }, client_name: "queue-e2e-test".to_string(), client_version: "0.1.0".to_string(), diff --git a/codex-rs/config/src/requirements_exec_policy.rs b/codex-rs/config/src/requirements_exec_policy.rs index 4bdd425236..7576d27c18 100644 --- a/codex-rs/config/src/requirements_exec_policy.rs +++ b/codex-rs/config/src/requirements_exec_policy.rs @@ -18,7 +18,7 @@ pub struct RequirementsExecPolicyToml { /// A TOML representation of the `prefix_rule(...)` Starlark builtin. /// -/// This mirrors the builtin defined in `execpolicy/src/parser.rs`. +/// This shares the builtin's matching semantics and additionally supports managed `prompt-user` decisions. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] pub struct RequirementsExecPolicyPrefixRuleToml { pub pattern: Vec, @@ -42,6 +42,7 @@ pub struct RequirementsExecPolicyPatternTokenToml { pub enum RequirementsExecPolicyDecisionToml { Allow, Prompt, + PromptUser, Forbidden, } @@ -50,6 +51,7 @@ impl RequirementsExecPolicyDecisionToml { match self { Self::Allow => Decision::Allow, Self::Prompt => Decision::Prompt, + Self::PromptUser => Decision::PromptUser, Self::Forbidden => Decision::Forbidden, } } @@ -79,7 +81,7 @@ pub enum RequirementsExecPolicyParseError { MissingDecision { rule_index: usize }, #[error( - "rules prefix_rule at index {rule_index} has decision 'allow', which is not permitted in requirements.toml: Codex merges these rules with other config and uses the most restrictive result (use 'prompt' or 'forbidden')" + "rules prefix_rule at index {rule_index} has decision 'allow', which is not permitted in requirements.toml: Codex merges these rules with other config and uses the most restrictive result (use 'prompt', 'prompt-user', or 'forbidden')" )] AllowDecisionNotAllowed { rule_index: usize }, } diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 7e307e4f4a..307adced35 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -406,9 +406,9 @@ impl ExecPolicyManager { ), ), }, - Decision::Prompt => { + Decision::Prompt | Decision::PromptUser => { let prompt_is_rule = evaluation.matched_rules.iter().any(|rule_match| { - is_policy_match(rule_match) && rule_match.decision() == Decision::Prompt + is_policy_match(rule_match) && rule_match.decision() == evaluation.decision }); match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) { Some(reason) if prompt_is_rule => ExecApprovalRequirement::Forbidden { @@ -425,6 +425,11 @@ impl ExecPolicyManager { ), ), }, + None if evaluation.decision == Decision::PromptUser => { + ExecApprovalRequirement::NeedsHumanApproval { + reason: derive_prompt_reason(command, &evaluation), + } + } None => ExecApprovalRequirement::NeedsApproval { reason: derive_prompt_reason(command, &evaluation), proposed_execpolicy_amendment: requested_amendment.or_else(|| { @@ -1048,20 +1053,27 @@ fn derive_prompt_reason(command_args: &[String], evaluation: &Evaluation) -> Opt .filter_map(|rule_match| match rule_match { RuleMatch::PrefixRuleMatch { matched_prefix, - decision: Decision::Prompt, + decision, justification, .. - } => Some((matched_prefix.len(), justification.as_deref())), + } if *decision == evaluation.decision => { + Some((matched_prefix.len(), justification.as_deref())) + } _ => None, }) .max_by_key(|(matched_prefix_len, _)| *matched_prefix_len); + let approval = if evaluation.decision == Decision::PromptUser { + "human approval" + } else { + "approval" + }; match most_specific_prompt { Some((_matched_prefix_len, Some(justification))) => { - Some(format!("`{command}` requires approval: {justification}")) + Some(format!("`{command}` requires {approval}: {justification}")) } Some((_matched_prefix_len, None)) => { - Some(format!("`{command}` requires approval by policy")) + Some(format!("`{command}` requires {approval} by policy")) } None => None, } diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index c17b910a26..6bb018831c 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -2554,6 +2554,7 @@ async fn windows_powershell_policy_classifies_only_independent_literal_commands( let actual = match requirement { ExecApprovalRequirement::Skip { .. } => Decision::Allow, ExecApprovalRequirement::NeedsApproval { .. } => Decision::Prompt, + ExecApprovalRequirement::NeedsHumanApproval { .. } => Decision::PromptUser, ExecApprovalRequirement::Forbidden { .. } => Decision::Forbidden, }; assert_eq!( diff --git a/codex-rs/core/src/guardian/recorded_tool_call.rs b/codex-rs/core/src/guardian/recorded_tool_call.rs index 65b8910de4..92df722076 100644 --- a/codex-rs/core/src/guardian/recorded_tool_call.rs +++ b/codex-rs/core/src/guardian/recorded_tool_call.rs @@ -58,6 +58,11 @@ pub(crate) async fn review_tool_call( let approval = match &requirement { ExecApprovalRequirement::Skip { .. } => return Ok(None), ExecApprovalRequirement::NeedsApproval { reason, .. } => reason.clone(), + ExecApprovalRequirement::NeedsHumanApproval { .. } => { + return Err(CodexErr::InvalidRequest( + "Managed command policy requires human approval.".into(), + )); + } ExecApprovalRequirement::Forbidden { reason } => { return Err(CodexErr::InvalidRequest(reason.clone())); } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 8c4ce1f5d0..eacf5aa59f 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1604,6 +1604,7 @@ async fn maybe_request_mcp_tool_approval( allow_persistent_approval: persistent_approval_key.is_some(), }; let approval_context = ApprovalContext { + exec_requirement: None, review_context: GuardianReviewContext::from(step_context), cancellation_token: Some(cancellation_token.clone()), call_id: call_id.to_string(), diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 128bc7d5bf..b66d272d6f 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -177,12 +177,44 @@ pub async fn resolve_elicitation( /// Propagate a user's exec approval decision to the session. /// Also optionally applies an execpolicy amendment. +#[expect( + clippy::await_holding_invalid_type, + reason = "bind a human decision to its active turn before resolving it" +)] pub async fn exec_approval( sess: &Arc, approval_id: String, turn_id: Option, decision: ReviewDecision, ) { + let human_approval = { + let active = sess.active_turn.lock().await; + let Some(turn) = active.as_ref() else { + return; + }; + let mut state = turn.turn_state.lock().await; + let Some(pending) = state.pending_approval(&approval_id) else { + return; + }; + if pending.kind == codex_protocol::approvals::ExecApprovalKind::HumanCommand { + state.remove_pending_approval(&approval_id) + } else { + None + } + }; + if let Some(pending) = human_approval { + let decision = pending.validate(decision); + if decision == ReviewDecision::Abort { + sess.abort_turn_if_active( + &pending.turn_id, + TurnAbortReason::Interrupted, + /*error*/ None, + ) + .await; + } + pending.sender.send(decision).ok(); + return; + } let event_turn_id = turn_id.unwrap_or_else(|| approval_id.clone()); if let ReviewDecision::ApprovedExecpolicyAmendment { proposed_execpolicy_amendment, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 9e9f51d46e..7fc2a27a00 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3107,7 +3107,12 @@ impl Session { match active.as_mut() { Some(at) => { let mut ts = at.turn_state.lock().await; - ts.insert_pending_approval(effective_approval_id.clone(), tx_approve) + ts.insert_pending_approval( + effective_approval_id.clone(), + tx_approve, + kind, + turn_context.sub_id.clone(), + ) } None => None, } @@ -3193,7 +3198,12 @@ impl Session { match active.as_mut() { Some(at) => { let mut ts = at.turn_state.lock().await; - ts.insert_pending_approval(approval_id.clone(), tx_approve) + ts.insert_pending_approval( + approval_id.clone(), + tx_approve, + ExecApprovalKind::Command, + turn_context.sub_id.clone(), + ) } None => None, } @@ -3270,6 +3280,7 @@ impl Session { let context = sandbox_context.policy_context(); { let approval_context = ApprovalContext { + exec_requirement: None, review_context: crate::guardian::GuardianReviewContext::from(step_context), cancellation_token: Some(cancellation_token.clone()), call_id: call_id.clone(), @@ -3623,8 +3634,9 @@ impl Session { } }; match entry { - Some(tx_approve) => { - tx_approve.send(decision).ok(); + Some(pending) => { + let decision = pending.validate(decision); + pending.sender.send(decision).ok(); } None => { warn!("No pending approval found for call_id: {approval_id}"); diff --git a/codex-rs/core/src/state/turn.rs b/codex-rs/core/src/state/turn.rs index 0fe0fea1bb..0eba857772 100644 --- a/codex-rs/core/src/state/turn.rs +++ b/codex-rs/core/src/state/turn.rs @@ -88,7 +88,7 @@ pub(crate) struct RunningTask { /// Mutable state for a single turn. #[derive(Default)] pub(crate) struct TurnState { - pending_approvals: HashMap>, + pending_approvals: HashMap, pending_request_permissions: HashMap, pending_user_input: HashMap>, pending_elicitations: HashMap<(String, RequestId), oneshot::Sender>, @@ -104,6 +104,27 @@ pub(crate) struct TurnState { pub(crate) last_known_step_context: Option>, } +pub(crate) struct PendingApproval { + pub(crate) sender: oneshot::Sender, + pub(crate) kind: codex_protocol::approvals::ExecApprovalKind, + pub(crate) turn_id: String, +} + +impl PendingApproval { + pub(crate) fn validate(&self, decision: ReviewDecision) -> ReviewDecision { + if self.kind == codex_protocol::approvals::ExecApprovalKind::HumanCommand + && !matches!( + decision, + ReviewDecision::Approved | ReviewDecision::Denied { .. } | ReviewDecision::Abort + ) + { + ReviewDecision::denied("Managed command policy requires a one-time human approval.") + } else { + decision + } + } +} + /// Host receipt metadata follows the response through the asynchronous tool waiter. pub(crate) struct AcceptedUserInputResponse { pub(crate) response: RequestUserInputResponse, @@ -124,14 +145,24 @@ impl TurnState { &mut self, key: String, tx: oneshot::Sender, - ) -> Option> { - self.pending_approvals.insert(key, tx) + kind: codex_protocol::approvals::ExecApprovalKind, + turn_id: String, + ) -> Option { + self.pending_approvals.insert( + key, + PendingApproval { + sender: tx, + kind, + turn_id, + }, + ) } - pub(crate) fn remove_pending_approval( - &mut self, - key: &str, - ) -> Option> { + pub(crate) fn pending_approval(&self, key: &str) -> Option<&PendingApproval> { + self.pending_approvals.get(key) + } + + pub(crate) fn remove_pending_approval(&mut self, key: &str) -> Option { self.pending_approvals.remove(key) } diff --git a/codex-rs/core/src/tools/approvals.rs b/codex-rs/core/src/tools/approvals.rs index 74082aaf49..e8461eea8a 100644 --- a/codex-rs/core/src/tools/approvals.rs +++ b/codex-rs/core/src/tools/approvals.rs @@ -1,4 +1,5 @@ //! Central approval policy-stage execution and reviewer routing. +//! Managed human checkpoints cannot be satisfied by hooks, automated review, or reusable approvals. use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec_policy::prompt_is_rejected_by_policy; @@ -16,6 +17,7 @@ use crate::tools::hook_names::HookToolName; use crate::tools::runtimes::apply_patch::ApplyPatchApprovalKey; use crate::tools::runtimes::unified_exec::UnifiedExecApprovalKey; use crate::tools::sandboxing::ApprovalRequestReasons; +use crate::tools::sandboxing::ExecApprovalRequirement; use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::with_cached_approval; @@ -51,6 +53,9 @@ use tracing::warn; #[derive(Clone)] pub(crate) struct ApprovalContext { + /// Preserve the command's policy result through review and retries. Other actions + /// use their existing reviewer routing without an execution policy requirement. + pub(crate) exec_requirement: Option, pub(crate) review_context: GuardianReviewContext, pub(crate) cancellation_token: Option, pub(crate) call_id: String, @@ -61,6 +66,15 @@ pub(crate) struct ApprovalContext { pub(crate) network_approval_context: Option, } +impl ApprovalContext { + fn requires_human_approval(&self) -> bool { + matches!( + self.exec_requirement, + Some(ExecApprovalRequirement::NeedsHumanApproval { .. }) + ) + } +} + #[derive(Clone, Debug, PartialEq, serde::Serialize)] #[serde(tag = "tool", rename_all = "snake_case")] pub(crate) enum ApprovalAction { @@ -494,11 +508,42 @@ impl ApprovalResolution { } impl Session { + async fn check_approval_requirement(&self, ctx: &ApprovalContext) -> Result<(), ToolError> { + if ctx.requires_human_approval() { + if let Some(reason) = prompt_is_rejected_by_policy( + ctx.review_context.approval_policy, + /*prompt_is_rule*/ true, + ) { + return Err(ToolError::Rejected(reason.to_string())); + } + let config = self.get_config().await; + let requirements = config.config_layer_stack.requirements(); + if ctx.strict_auto_review + || ctx.review_context.turn().strict_auto_review_enabled() + || requirements + .approvals_reviewer + .can_set(&ApprovalsReviewer::User) + .is_err() + || requirements.auto_review_required_for_model(&ctx.review_context.model_info.slug) + { + return Err(ToolError::Rejected("Managed command policy requires human approval, but this session requires automatic review.".to_string())); + } + } + Ok(()) + } + pub(crate) async fn request_approval( self: &Arc, action: ApprovalAction, ctx: ApprovalContext, ) -> ApprovalResult { + if let Err(err) = self.check_approval_requirement(&ctx).await { + return ApprovalResult { + result: Err(err), + source: None, + cancellation_reason: None, + }; + } // Retained authority outside current grants needs a new sandbox approval. // Current grants permit review of the input; they never approve the input itself. let policy = ctx.review_context.turn().approval_policy(); @@ -523,10 +568,9 @@ impl Session { _ => ctx.call_id.clone(), }; - // Approval precedence is: - // 1. Hooks - // 2. If StrictAutoReview || Guardian enabled, then Guardian. Else, user. - let resolution = match run_permission_request_hooks( + // Hook denials always win. Managed checkpoints require a person even + // when a hook allows the action or the session uses automatic review. + let mut resolution = match run_permission_request_hooks( self, &ctx.review_context, &permission_request_run_id, @@ -535,18 +579,35 @@ impl Session { ) .await { - Some(PermissionRequestDecision::Allow) => ApprovalResolution { - decision: ReviewDecision::Approved, - source: ApprovalResolutionSource::Hook, - cancellation_reason: None, - }, + Some(PermissionRequestDecision::Allow) if !ctx.requires_human_approval() => { + ApprovalResolution { + decision: ReviewDecision::Approved, + source: ApprovalResolutionSource::Hook, + cancellation_reason: None, + } + } Some(PermissionRequestDecision::Deny { message }) => ApprovalResolution { decision: ReviewDecision::denied(message), source: ApprovalResolutionSource::Hook, cancellation_reason: None, }, - None => self.request_reviewer_approval(action, &ctx).await, + Some(PermissionRequestDecision::Allow) | None => { + self.request_reviewer_approval(action, &ctx).await + } }; + if resolution.decision == ReviewDecision::Approved { + if let Err(ToolError::Rejected(reason)) = self.check_approval_requirement(&ctx).await { + resolution.decision = ReviewDecision::denied(reason); + } + if ctx.requires_human_approval() + && ctx + .cancellation_token + .as_ref() + .is_some_and(CancellationToken::is_cancelled) + { + resolution.decision = ReviewDecision::Abort; + } + } // Network approvals record their final telemetry after validation and persistence. if !is_network_approval { record_resolution(&ctx, &resolution); @@ -598,7 +659,9 @@ impl Session { action: ApprovalAction, ctx: &ApprovalContext, ) -> ApprovalResolution { - if let Some(decision) = self.request_guardian_approval(action.clone(), ctx).await { + if ctx.requires_human_approval() { + self.request_user_approval(&action, ctx).await + } else if let Some(decision) = self.request_guardian_approval(action.clone(), ctx).await { ApprovalResolution { cancellation_reason: (decision == ReviewDecision::Abort) .then_some(McpApprovalCancellationReason::Interrupted), @@ -705,45 +768,68 @@ impl Session { proposed_execpolicy_amendment, .. } => { - let tool_name = "unified_exec"; let reason = ctx .retry_reason .clone() .or_else(|| ctx.approval_reason.clone()) .or_else(|| justification.clone()); - let policy_fingerprint = ctx - .review_context - .environments() - .turn_environments() - .find(|environment| environment.selection.environment_id == *environment_id) - .and_then(|environment| environment.config().exec_policy.as_ref()) - .map(codex_execpolicy::RequirementsExecPolicy::fingerprint); - let cache_keys = action - .cache_keys() - .into_iter() - .map(|key| (key, &policy_fingerprint)) - .collect(); - with_cached_approval(&self.services, tool_name, cache_keys, || async { + let human_once = ctx.requires_human_approval(); + let request = || async { self.request_command_approval( ctx.review_context.turn(), - ExecApprovalKind::Command, + if human_once { + ExecApprovalKind::HumanCommand + } else { + ExecApprovalKind::Command + }, ctx.review_context.model_context(), ctx.call_id.clone(), Some(&ctx.call_id), - /*approval_id*/ None, + human_once.then(|| uuid::Uuid::new_v4().to_string()), Some(environment_id.clone()), command.clone(), cwd.clone(), reason, - ctx.network_approval_context.clone(), - proposed_execpolicy_amendment.clone(), + if human_once { + None + } else { + ctx.network_approval_context.clone() + }, + if human_once { + None + } else { + proposed_execpolicy_amendment.clone() + }, additional_permissions.clone(), - /*available_decisions*/ None, + human_once.then(|| { + vec![ + ReviewDecision::Approved, + ReviewDecision::denied("rejected by user"), + ReviewDecision::Abort, + ] + }), /*plugin_attribution_override*/ None, ) .await - }) - .await + }; + if human_once { + request().await + } else { + let tool_name = "unified_exec"; + let policy_fingerprint = ctx + .review_context + .environments() + .turn_environments() + .find(|environment| environment.selection.environment_id == *environment_id) + .and_then(|environment| environment.config().exec_policy.as_ref()) + .map(codex_execpolicy::RequirementsExecPolicy::fingerprint); + let cache_keys = action + .cache_keys() + .into_iter() + .map(|key| (key, &policy_fingerprint)) + .collect(); + with_cached_approval(&self.services, tool_name, cache_keys, request).await + } } ApprovalAction::WriteStdin { id, diff --git a/codex-rs/core/src/tools/approvals_tests.rs b/codex-rs/core/src/tools/approvals_tests.rs index 3620a3cd81..12c49cff64 100644 --- a/codex-rs/core/src/tools/approvals_tests.rs +++ b/codex-rs/core/src/tools/approvals_tests.rs @@ -128,6 +128,7 @@ async fn non_utf8_cwd_preserves_approval_routing( review_context.approval_policy = AskForApproval::OnRequest; review_context.approvals_reviewer = reviewer; let context = ApprovalContext { + exec_requirement: None, review_context, cancellation_token: None, call_id: "non-utf8-cwd".to_string(), @@ -213,6 +214,7 @@ async fn explicit_mcp_reviewer_override_takes_precedence_over_action_context() { review_context.approval_policy = AskForApproval::OnRequest; review_context.approvals_reviewer = ApprovalsReviewer::AutoReview; let context = ApprovalContext { + exec_requirement: None, review_context, cancellation_token: None, call_id: "mcp-override".to_string(), diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 5971d10683..4b3a5b5aba 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -799,6 +799,7 @@ impl NetworkApprovalService { cwd, }; let approval_context = ApprovalContext { + exec_requirement: None, review_context, cancellation_token: None, call_id: approval_call_id, diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index a1a88e4454..809e43bc2e 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -164,6 +164,10 @@ impl ToolOrchestrator { let requirement = tool.exec_approval_requirement(req).unwrap_or_else(|| { default_exec_approval_requirement(approval_policy, &file_system_sandbox_policy) }); + let human_once = matches!( + requirement, + ExecApprovalRequirement::NeedsHumanApproval { .. } + ); match &requirement { ExecApprovalRequirement::Skip { .. } => { if strict_auto_review { @@ -173,6 +177,7 @@ impl ToolOrchestrator { ToolError::Rejected(format!("could not prepare approval action: {err}")) })?; let approval_ctx = ApprovalContext { + exec_requirement: Some(requirement.clone()), review_context: GuardianReviewContext::from(&tool_ctx.step_context), cancellation_token: Some(tool_ctx.cancellation_token.clone()), call_id: tool_ctx.call_id.clone(), @@ -200,13 +205,15 @@ impl ToolOrchestrator { ExecApprovalRequirement::Forbidden { reason } => { return Err(ToolError::Rejected(reason.clone())); } - ExecApprovalRequirement::NeedsApproval { reason, .. } => { + ExecApprovalRequirement::NeedsApproval { reason, .. } + | ExecApprovalRequirement::NeedsHumanApproval { reason } => { let action = tool .approval_action(req, &tool_ctx.call_id) .map_err(|err| { ToolError::Rejected(format!("could not prepare approval action: {err}")) })?; let approval_ctx = ApprovalContext { + exec_requirement: Some(requirement.clone()), review_context: GuardianReviewContext::from(&tool_ctx.step_context), cancellation_token: Some(tool_ctx.cancellation_token.clone()), call_id: tool_ctx.call_id.clone(), @@ -483,16 +490,19 @@ impl ToolOrchestrator { // Human and policy approvals keep their existing retry scope. A // Guardian verdict only covers the authority of the reviewed action. - let retry_preapproved = matches!( - initial_approval_source, - Some(ApprovalResolutionSource::User | ApprovalResolutionSource::Hook) - ); - let bypass_retry_approval = !strict_auto_review + let retry_preapproved = !human_once + && matches!( + initial_approval_source, + Some(ApprovalResolutionSource::User | ApprovalResolutionSource::Hook) + ); + let bypass_retry_approval = !human_once + && !strict_auto_review && tool.should_bypass_approval(approval_policy, retry_preapproved) && network_approval_context.is_none(); if !bypass_retry_approval { let approval_reason = match &requirement { - ExecApprovalRequirement::NeedsApproval { reason, .. } => reason.clone(), + ExecApprovalRequirement::NeedsApproval { reason, .. } + | ExecApprovalRequirement::NeedsHumanApproval { reason } => reason.clone(), ExecApprovalRequirement::Skip { .. } | ExecApprovalRequirement::Forbidden { .. } => None, }; @@ -516,6 +526,7 @@ impl ToolOrchestrator { *additional_permissions = None; } let approval_ctx = ApprovalContext { + exec_requirement: Some(requirement.clone()), review_context: GuardianReviewContext::from(&tool_ctx.step_context), cancellation_token: Some(tool_ctx.cancellation_token.clone()), call_id: tool_ctx.call_id.clone(), @@ -526,6 +537,15 @@ impl ToolOrchestrator { network_approval_context: network_approval_context.clone(), }; + if human_once && network_approval_context.is_some() { + let mut network_ctx = approval_ctx.clone(); + network_ctx.exec_requirement = None; + tool_ctx + .session + .request_approval(action.clone(), network_ctx) + .await + .result?; + } tool_ctx .session .request_approval(action, approval_ctx) diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 5a51a5dc6b..d80079d307 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -170,6 +170,8 @@ pub(crate) enum ExecApprovalRequirement { /// See core/src/exec_policy.rs for more details on how proposed_execpolicy_amendment is determined. proposed_execpolicy_amendment: Option, }, + /// A fresh human decision is required for each execution attempt. + NeedsHumanApproval { reason: Option }, /// Execution forbidden for this tool call. Forbidden { reason: String }, } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 2a2eb2afe0..9715d1b1a8 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -972,6 +972,7 @@ impl UnifiedExecProcessManager { return Err(unreviewable_input_error()); } let approval_context = ApprovalContext { + exec_requirement: None, review_context: GuardianReviewContext::from(&context.step_context), cancellation_token: Some(context.cancellation_token.clone()), call_id: context.call_id.clone(), diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 06d51e3b00..08b86e18f1 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -720,7 +720,7 @@ elif mode == "exit_2": Ok(()) } -fn install_allow_permission_request_hook(home: &Path) -> Result<()> { +pub(super) fn install_allow_permission_request_hook(home: &Path) -> Result<()> { write_permission_request_hook( home, Some(PERMISSION_REQUEST_HOOK_MATCHER), @@ -1183,7 +1183,7 @@ fn read_pre_tool_use_hook_inputs(home: &Path) -> Result> read_hook_inputs_from_log(home.join("pre_tool_use_hook_log.jsonl").as_path()) } -fn read_permission_request_hook_inputs(home: &Path) -> Result> { +pub(super) fn read_permission_request_hook_inputs(home: &Path) -> Result> { fs::read_to_string(home.join("permission_request_hook_log.jsonl")) .context("read permission request hook log")? .lines() diff --git a/codex-rs/core/tests/suite/scenarios.rs b/codex-rs/core/tests/suite/scenarios.rs index 2d0c67d8bb..b5410da20f 100644 --- a/codex-rs/core/tests/suite/scenarios.rs +++ b/codex-rs/core/tests/suite/scenarios.rs @@ -123,6 +123,9 @@ mod model_instructions; #[path = "scenarios_catalog_approval_instructions_tests.rs"] mod catalog_approval_instructions; +#[path = "scenarios_managed_human_approval_tests.rs"] +mod managed_human_approval; + #[path = "scenarios_context_management_tools.rs"] mod context_management_tools; diff --git a/codex-rs/core/tests/suite/scenarios_managed_human_approval_tests.rs b/codex-rs/core/tests/suite/scenarios_managed_human_approval_tests.rs new file mode 100644 index 0000000000..8176988d8d --- /dev/null +++ b/codex-rs/core/tests/suite/scenarios_managed_human_approval_tests.rs @@ -0,0 +1,182 @@ +#![cfg(test)] + +//! Managed checkpoints require a fresh human decision even with automatic review, hooks, and saved rules. + +#[cfg(not(target_os = "windows"))] +use super::super::hooks::install_allow_permission_request_hook; +#[cfg(not(target_os = "windows"))] +use super::super::hooks::read_permission_request_hook_inputs; +use super::*; +use anyhow::Context; +use codex_config::test_support::CloudConfigBundleFixture; +use codex_core::config::Constrained; +use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::models::PermissionProfile; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::ReviewDecision; +#[cfg(not(target_os = "windows"))] +use core_test_support::hooks::trust_discovered_hooks; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::mount_sse_once; +#[cfg(not(target_os = "windows"))] +use pretty_assertions::assert_eq; +use tokio_util::sync::CancellationToken; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn managed_checkpoint_requires_fresh_human_decisions() -> Result<()> { + skip_if_no_network!(Ok(())); + skip_if_wine_exec!(Ok(()), "command approval requires host-native paths"); + let server = start_mock_server().await; + let builder = test_codex(); + #[cfg(not(target_os = "windows"))] + let builder = builder.with_pre_build_hook(|home| { + install_allow_permission_request_hook(home).expect("allow permission request hook"); + }); + let test = builder + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#" +allowed_approvals_reviewers = ["auto_review", "user"] +[[rules.prefix_rules]] +pattern = [{ token = "echo" }] +decision = "prompt-user" +justification = "A person must approve this command." +"#, + ), + ) + .with_config(|config| { + configure_scenario_catalog(config); + #[cfg(not(target_os = "windows"))] + trust_discovered_hooks(config); + config + .features + .enable(Feature::UnifiedExec) + .expect("unified exec"); + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config + .permissions + .set_permission_profile(PermissionProfile::Disabled) + .expect("permissions"); + config.approvals_reviewer = ApprovalsReviewer::AutoReview; + fs::create_dir_all(config.codex_home.join("rules")).expect("rules directory"); + fs::write( + config.codex_home.join("rules/allow.rules"), + r#"prefix_rule(pattern=["echo"], decision="allow")"#, + ) + .expect("saved allowance"); + }) + .build_with_auto_env(&server) + .await?; + let mut requests = Vec::new(); + let mut approval_ids = HashSet::::new(); + for (index, decision) in [ + ReviewDecision::ApprovedForSession, + ReviewDecision::Approved, + ReviewDecision::Approved, + ] + .into_iter() + .enumerate() + { + let call_id = format!("command-{index}"); + let command = mount_sse_once( + &server, + sse(vec![ + ev_response_created(&call_id), + ev_function_call( + &call_id, + "exec_command", + &json!({"cmd": "echo checkpoint-ok", "login": false, "yield_time_ms": 10000}) + .to_string(), + ), + ev_completed(&call_id), + ]), + ) + .await; + let continuation = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("done", "Done."), + ev_completed("done"), + ]), + ) + .await; + test.codex + .start_or_steer_turn(TurnInputRequest::user_input(vec![text("Run the command.")])) + .await?; + let approval = wait_for_event_match(&test.codex, |event| match event { + EventMsg::ExecApprovalRequest(approval) => Some(approval.clone()), + _ => None, + }) + .await; + #[cfg(not(target_os = "windows"))] + assert_eq!( + read_permission_request_hook_inputs(test.codex_home_path())?.len(), + index + 1, + ); + let id = approval + .approval_id + .context("human approval has a unique id")?; + assert!(approval_ids.insert(id.clone())); + test.codex + .submit(Op::ExecApproval { + id, + turn_id: None, + decision, + }) + .await?; + requests.push(command.single_request()); + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + let result = continuation.single_request(); + let output = result + .function_call_output_text(&call_id) + .context("command result")?; + if index == 0 { + assert!(output.contains("one-time human approval"), "{output}"); + } else { + assert!(output.contains("checkpoint-ok"), "{output}"); + } + requests.push(result); + } + // Direct Guardian review cannot substitute for the managed human checkpoint either. + test.codex + .inject_response_items(vec![serde_json::from_value(json!({ + "type": "function_call", "name": "exec_command", "call_id": "direct-review", + "arguments": r#"{"cmd":"echo checkpoint-ok","login":false}"#, + }))?]) + .await?; + let error = test + .codex + .guardian_review(&CancellationToken::new()) + .await + .expect_err("managed checkpoint requires a human"); + assert!(matches!( + error.details(), + codex_protocol::error::CodexErrorDetails::InvalidRequest(message) + if message == "Managed command policy requires human approval." + )); + let mut snapshot = context_snapshot::format_request_history_snapshot( + "Managed command checkpoints override saved allow rules and automatic review. Reusable decisions are rejected, and each successful execution requires a fresh human decision.", + &requests, + &ContextSnapshotOptions::default().rewrite_known_segments(), + ); + for (pattern, replacement) in [ + (r"Chunk ID: [a-z0-9]+", "Chunk ID: "), + ( + r"Wall time: [0-9]+(?:\.[0-9]+)? seconds", + "Wall time: seconds", + ), + ( + r"Original token count: [0-9]+", + "Original token count: ", + ), + ] { + snapshot = regex_lite::Regex::new(pattern)? + .replace_all(&snapshot, replacement) + .into_owned(); + } + insta::assert_snapshot!(snapshot); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__managed_human_approval__managed_checkpoint_requires_fresh_human_decisions.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__managed_human_approval__managed_checkpoint_requires_fresh_human_decisions.snap new file mode 100644 index 0000000000..723d54493b --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__managed_human_approval__managed_checkpoint_requires_fresh_human_decisions.snap @@ -0,0 +1,46 @@ +--- +source: core/tests/suite/scenarios_managed_human_approval_tests.rs +expression: snapshot +--- +Scenario: Managed command checkpoints override saved allow rules and automatic review. Reusable decisions are rejected, and each successful execution requires a fresh human decision. + +## Window 1 +-- request 1 (turn) -- +00:message/developer: + +01:message/developer: + +02:message/user: + +03:message/user: + Run the command. +-- request 2 (turn) -- +04:function_call/exec_command:{"cmd":"echo checkpoint-ok","login":false,"yield_time_ms":10000} +05:function_call_output:exec_command failed: CreateProcess { message: "Rejected(\\"Managed command policy requires a one-time human approval.\\")" } +-- request 3 (turn) -- +06:message/assistant: + Done. +07:message/user: + Run the command. +-- request 4 (turn) -- +08:function_call/exec_command:{"cmd":"echo checkpoint-ok","login":false,"yield_time_ms":10000} +09:function_call_output:Chunk ID: + Wall time: seconds + Process exited with code 0 + Original token count: + Output: + checkpoint-ok + +-- request 5 (turn) -- +10:message/assistant: + Done. +11:message/user: + Run the command. +-- request 6 (turn) -- +12:function_call/exec_command:{"cmd":"echo checkpoint-ok","login":false,"yield_time_ms":10000} +13:function_call_output:Chunk ID: + Wall time: seconds + Process exited with code 0 + Original token count: + Output: + checkpoint-ok diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 43cf70b21d..7f71e60b84 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -735,6 +735,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result .await? }; let in_process_start_args = InProcessClientStartArgs { + required_human_approval: false, arg0_paths, config: std::sync::Arc::new(config.clone()), cli_overrides: run_cli_overrides, diff --git a/codex-rs/exec/src/worktree.rs b/codex-rs/exec/src/worktree.rs index a61d173dfa..317a2da07f 100644 --- a/codex-rs/exec/src/worktree.rs +++ b/codex-rs/exec/src/worktree.rs @@ -28,6 +28,7 @@ pub(super) async fn fork_source( ) .await?; let client = InProcessAppServerClient::start(InProcessClientStartArgs { + required_human_approval: false, arg0_paths: arg0_paths.clone(), config: std::sync::Arc::new(config.clone()), cli_overrides: cli_overrides.to_vec(), diff --git a/codex-rs/execpolicy/src/amend.rs b/codex-rs/execpolicy/src/amend.rs index e25fd1bd91..2716fbf81b 100644 --- a/codex-rs/execpolicy/src/amend.rs +++ b/codex-rs/execpolicy/src/amend.rs @@ -106,6 +106,11 @@ pub fn blocking_append_network_rule( let decision = serde_json::to_string(match decision { Decision::Allow => "allow", Decision::Prompt => "prompt", + Decision::PromptUser => { + return Err(AmendError::InvalidNetworkRule( + "prompt-user is only supported in managed command rules".to_string(), + )); + } Decision::Forbidden => "deny", }) .map_err(|source| AmendError::SerializeNetworkRule { source })?; diff --git a/codex-rs/execpolicy/src/decision.rs b/codex-rs/execpolicy/src/decision.rs index 15dc1f711d..0c8933c0cb 100644 --- a/codex-rs/execpolicy/src/decision.rs +++ b/codex-rs/execpolicy/src/decision.rs @@ -9,8 +9,11 @@ use crate::error::Result; pub enum Decision { /// Command may run without further approval. Allow, - /// Request explicit user approval; rejected outright when running with `approval_policy="never"`. + /// Request approval from the configured reviewer; rejected with `approval_policy="never"`. Prompt, + /// Requires a fresh human decision, independent of the configured reviewer. + #[serde(rename = "prompt-user")] + PromptUser, /// Command is blocked without further consideration. Forbidden, } diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index 78790517fd..0bd1190f76 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -215,7 +215,7 @@ impl Policy { allowed.retain(|entry| entry != &rule.host); upsert_domain(&mut denied, &rule.host); } - Decision::Prompt => {} + Decision::Prompt | Decision::PromptUser => {} } } diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index f740befc93..b651d55ee7 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -275,6 +275,8 @@ pub struct GuardianAssessmentEvent { pub enum ExecApprovalKind { #[default] Command, + /// Requires a fresh human decision; automated and reusable approvals are invalid. + HumanCommand, WriteStdin, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 8c419c536c..62d01f556d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1748,7 +1748,9 @@ impl EventMsg { pub fn command_approval_start(&self) -> Option<(&str, i64)> { match self { Self::ExecApprovalRequest(request) - if request.approval_id.is_none() && request.network_approval_context.is_none() => + if request.kind == crate::approvals::ExecApprovalKind::Command + && request.approval_id.is_none() + && request.network_approval_context.is_none() => { Some((&request.call_id, request.started_at_ms)) } diff --git a/codex-rs/tui/src/analytics/chats_tests.rs b/codex-rs/tui/src/analytics/chats_tests.rs index 09a2de636c..c852cbc749 100644 --- a/codex-rs/tui/src/analytics/chats_tests.rs +++ b/codex-rs/tui/src/analytics/chats_tests.rs @@ -106,6 +106,7 @@ async fn slow_repairing_chat_listing_keeps_estimate_budget_and_ranks_across_page pages }); let remote = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: true, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url, auth_token: None, diff --git a/codex-rs/tui/src/analytics/connection_tests.rs b/codex-rs/tui/src/analytics/connection_tests.rs index a9c04e6a1f..b09c1c550a 100644 --- a/codex-rs/tui/src/analytics/connection_tests.rs +++ b/codex-rs/tui/src/analytics/connection_tests.rs @@ -48,6 +48,7 @@ async fn remote() -> (RemoteAppServerClient, tokio::task::JoinHandle<()>) { assert!(socket.next().await.unwrap().unwrap().is_close()); }); let client = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: true, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url: format!("ws://{address}"), auth_token: None, diff --git a/codex-rs/tui/src/analytics/tasks_tests.rs b/codex-rs/tui/src/analytics/tasks_tests.rs index 9bfcc4b538..bc1a6b1f31 100644 --- a/codex-rs/tui/src/analytics/tasks_tests.rs +++ b/codex-rs/tui/src/analytics/tasks_tests.rs @@ -87,6 +87,7 @@ async fn consumer_roots_include_paginated_archived_descendants_and_keep_missing_ pages }); let remote = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: true, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url, auth_token: None, diff --git a/codex-rs/tui/src/app_server_connection.rs b/codex-rs/tui/src/app_server_connection.rs index ac3ab1cb2c..31d6f528d2 100644 --- a/codex-rs/tui/src/app_server_connection.rs +++ b/codex-rs/tui/src/app_server_connection.rs @@ -29,6 +29,7 @@ pub(crate) async fn connect(target: &AppServerTarget) -> color_eyre::Result color_eyre::Result { let app_server = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: true, endpoint, client_name: "codex-tui".to_string(), client_version: env!("CARGO_PKG_VERSION").to_string(), @@ -741,6 +742,7 @@ where }) .collect(); let client = start_client(InProcessClientStartArgs { + required_human_approval: true, arg0_paths, config: Arc::new(config), cli_overrides: cli_kv_overrides, diff --git a/codex-rs/tui/src/named_session_lookup_tests.rs b/codex-rs/tui/src/named_session_lookup_tests.rs index 994256b8a8..cc871ea9f5 100644 --- a/codex-rs/tui/src/named_session_lookup_tests.rs +++ b/codex-rs/tui/src/named_session_lookup_tests.rs @@ -528,6 +528,7 @@ async fn verifies_name_uniqueness_across_server_pages() -> color_eyre::Result<() } }); let client = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { + required_human_approval: true, endpoint: RemoteAppServerEndpoint::WebSocket { websocket_url: url, auth_token: None, diff --git a/codex-rs/tui/src/onboarding/auth.rs b/codex-rs/tui/src/onboarding/auth.rs index 55ca227956..07c3629b2d 100644 --- a/codex-rs/tui/src/onboarding/auth.rs +++ b/codex-rs/tui/src/onboarding/auth.rs @@ -1285,6 +1285,7 @@ mod tests { .unwrap(); let mut auth_config = config.auth_config(); let client = InProcessAppServerClient::start(InProcessClientStartArgs { + required_human_approval: true, arg0_paths: Arg0DispatchPaths::default(), config: Arc::new(config), cli_overrides: Vec::new(), diff --git a/sdk/python/src/openai_codex/generated/v2_all.py b/sdk/python/src/openai_codex/generated/v2_all.py index 82a5c8c088..cd74114fe1 100644 --- a/sdk/python/src/openai_codex/generated/v2_all.py +++ b/sdk/python/src/openai_codex/generated/v2_all.py @@ -2126,6 +2126,13 @@ class InitializeCapabilities(BaseModel): description="Opt into `attestation/generate` requests for upstream `x-oai-attestation`.", ), ] = False + required_human_approval: Annotated[ + bool | None, + Field( + alias="requiredHumanApproval", + description="Present `humanCommand` approvals to a person and return only a one-time decision. Requires `experimentalApi`. Pending requests can be replayed to compatible clients.", + ), + ] = None class InitializeParams(BaseModel):