openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-19 17:28:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enforce non-interactive approval policy for Codex delegates (#38205)

Browse Source 
## What changed

- Require delegated Codex sessions to use the `never` approval policy and reject
  delegate creation with any prompt-capable policy.
- Stop forwarding delegate approval and permission requests to the parent
  session. Approval-requiring commands and MCP tool calls are denied within the
  delegate instead.
- Skip prompts for missing skill MCP dependencies when approvals are disabled.

## Testing

Add coverage for review and Guardian delegates to verify that command
escalations, MCP approvals, and skill dependency installation do not prompt or
execute approval-gated actions.

GitOrigin-RevId: fee96a97624352cd25c904592ba7eebe2ae3fd63
This commit is contained in:
Dylan Hurd
2026-08-12 15:31:47 +00:00
committed by copyberry
parent 3d7f9b4637
commit 95aada11c4
11 changed files with 483 additions and 1199 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/codex_delegate.rs
+13 -630
View File
@@ -1,55 +1,26 @@
use std::collections::HashMap;
use std::sync::Arc;
use async_channel::Receiver;
use async_channel::Sender;
use codex_analytics::GuardianApprovalRequestSource;
use codex_async_utils::OrCancelExt;
use codex_core_plugins::PluginCommandAttribution;
use codex_extension_api::LoadedUserInstructions;
use codex_plugin::PluginId;
use codex_protocol::items::is_safe_plugin_relative_path;
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
use codex_protocol::protocol::McpInvocation;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::RequestUserInputEvent;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::Submission;
use codex_protocol::protocol::ThreadSource;
use codex_protocol::request_permissions::PermissionGrantScope;
use codex_protocol::request_permissions::RequestPermissionsArgs;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_protocol::user_input::UserInput;
use serde_json::Value;
use std::time::Duration;
use tokio::sync::Mutex;
use tokio::sync::oneshot;
use tokio::time::timeout;
use tokio_util::sync::CancellationToken;
use crate::config::Config;
use crate::config::Constrained;
use crate::environment_selection::TurnEnvironmentSnapshot;
use crate::guardian::GuardianApprovalRequest;
use crate::guardian::GuardianReviewOptions;
use crate::guardian::new_guardian_review_id;
use crate::guardian::routes_approval_to_guardian;
use crate::guardian::routes_approval_to_guardian_with_reviewer;
use crate::guardian::spawn_approval_request_review;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC;
use crate::mcp_tool_call::McpToolApprovalMetadata;
use crate::mcp_tool_call::build_guardian_mcp_tool_review_request;
use crate::mcp_tool_call::is_mcp_tool_approval_question_id;
use crate::mcp_tool_call::mcp_approvals_reviewer;
use crate::session::ForkPersistence;
use crate::session::GitEnrichmentPolicy;
use crate::session::SUBMISSION_CHANNEL_CAPACITY;
@@ -67,20 +38,13 @@ use codex_protocol::protocol::MultiAgentVersion;
#[cfg(test)]
use crate::session::completed_session_loop_termination;
#[derive(Clone)]
struct PendingMcpInvocation {
invocation: McpInvocation,
metadata: Option<McpToolApprovalMetadata>,
}
/// Start an interactive sub-Codex thread and return its runtime and IO channels.
///
/// The returned IO yields non-approval events emitted by the sub-agent.
/// Approval requests are handled via `parent_session` and are not surfaced.
/// Delegates never request approvals, and the returned IO yields their public events.
/// Its submission channel accepts additional `Op`s for the sub-agent.
#[allow(clippy::too_many_arguments)]
pub(crate) async fn run_codex_thread_interactive(
config: Config,
mut config: Config,
auth_manager: Arc<AuthManager>,
models_manager: SharedModelsManager,
parent_session: Arc<Session>,
@@ -92,6 +56,13 @@ pub(crate) async fn run_codex_thread_interactive(
git_enrichment_policy: GitEnrichmentPolicy,
windows_sandbox_proxy_settings_mode: codex_sandboxing::WindowsSandboxProxySettingsMode,
) -> Result<(Arc<Session>, SessionIo), CodexErr> {
if config.permissions.approval_policy.value() != AskForApproval::Never {
return Err(CodexErr::InvalidRequest(
"Codex delegates require approval policy `never`".to_string(),
));
}
config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
let (tx_ops, rx_ops) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
let conversation_history = initial_history.unwrap_or(InitialHistory::New);
@@ -160,16 +131,8 @@ pub(crate) async fn run_codex_thread_interactive(
let cancel_token_events = cancel_token.child_token();
let cancel_token_ops = cancel_token.child_token();
// Forward events from the sub-agent to the consumer, filtering approvals and
// routing them to the parent session for decisions.
let parent_session_clone = Arc::clone(&parent_session);
let parent_ctx_clone = Arc::clone(&parent_ctx);
let session_for_events = Arc::clone(&session);
// Forward public events from the sub-agent to the consumer.
let io = Arc::new(io);
// Cache the child call's MCP metadata at begin time. The later legacy
// RequestUserInput approval event only carries a call_id and question metadata.
let pending_mcp_invocations =
Arc::new(Mutex::new(HashMap::<String, PendingMcpInvocation>::new()));
let caller_io = SessionIo {
tx_sub: tx_ops,
rx_event: rx_sub,
@@ -178,16 +141,7 @@ pub(crate) async fn run_codex_thread_interactive(
};
let io_for_events = Arc::clone(&io);
tokio::spawn(async move {
forward_events(
io_for_events,
session_for_events,
tx_sub,
parent_session_clone,
parent_ctx_clone,
pending_mcp_invocations,
cancel_token_events,
)
.await;
forward_events(io_for_events, tx_sub, cancel_token_events).await;
});
// Forward ops from the caller to the sub-agent.
@@ -296,11 +250,7 @@ pub(crate) async fn run_codex_thread_one_shot(
async fn forward_events(
io: Arc<SessionIo>,
session: Arc<Session>,
tx_sub: Sender<Event>,
parent_session: Arc<Session>,
parent_ctx: Arc<TurnContext>,
pending_mcp_invocations: Arc<Mutex<HashMap<String, PendingMcpInvocation>>>,
cancel_token: CancellationToken,
) {
let cancelled = cancel_token.cancelled();
@@ -326,117 +276,6 @@ async fn forward_events(
| EventMsg::McpStartupUpdate(_)
| EventMsg::McpStartupComplete(_),
} => {}
Event {
id,
msg: EventMsg::ExecApprovalRequest(event),
} => {
// Initiate approval via parent session; do not surface to consumer.
handle_exec_approval(
&io,
id,
&parent_session,
&parent_ctx,
event,
&cancel_token,
)
.await;
}
Event {
id,
msg: EventMsg::ApplyPatchApprovalRequest(event),
} => {
handle_patch_approval(
&io,
id,
&parent_session,
&parent_ctx,
event,
&cancel_token,
)
.await;
}
Event {
msg: EventMsg::RequestPermissions(event),
..
} => {
handle_request_permissions(
&io,
&parent_session,
&parent_ctx,
event,
&cancel_token,
)
.await;
}
Event {
id,
msg: EventMsg::RequestUserInput(event),
} => {
handle_request_user_input(
&io,
id,
&parent_session,
&parent_ctx,
&pending_mcp_invocations,
event,
&cancel_token,
)
.await;
}
Event {
id,
msg: EventMsg::McpToolCallBegin(event),
} => {
// The later approval event has only a call ID. Retain the exact facts
// captured before this begin event instead of consulting the latest
// runtime after a refresh.
let metadata = session
.mcp_tool_approval_metadata(&id, &event.call_id)
.await
.map(|(_, metadata)| metadata);
pending_mcp_invocations
.lock()
.await
.insert(
event.call_id.clone(),
PendingMcpInvocation {
invocation: event.invocation.clone(),
metadata,
},
);
if !forward_event_or_shutdown(
&io,
&tx_sub,
&cancel_token,
Event {
id,
msg: EventMsg::McpToolCallBegin(event),
},
)
.await
{
break;
}
}
Event {
id,
msg: EventMsg::McpToolCallEnd(event),
} => {
pending_mcp_invocations.lock().await.remove(&event.call_id);
if !forward_event_or_shutdown(
&io,
&tx_sub,
&cancel_token,
Event {
id,
msg: EventMsg::McpToolCallEnd(event),
},
)
.await
{
break;
}
}
other => {
if !forward_event_or_shutdown(&io, &tx_sub, &cancel_token, other).await
{
@@ -497,462 +336,6 @@ async fn forward_ops(
}
}
/// Handle an ExecApprovalRequest by consulting the parent session and replying.
async fn handle_exec_approval(
io: &SessionIo,
turn_id: String,
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
event: ExecApprovalRequestEvent,
cancel_token: &CancellationToken,
) {
let approval_id_for_op = event.effective_approval_id();
let ExecApprovalRequestEvent {
call_id,
plugin_id,
script_path,
approval_id,
environment_id,
command,
cwd,
reason,
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
available_decisions,
..
} = event;
let plugin_attribution = plugin_id
.zip(script_path)
.and_then(|(plugin_id, script_path)| {
let plugin_id = PluginId::parse(&plugin_id).ok()?;
is_safe_plugin_relative_path(&script_path).then_some(PluginCommandAttribution {
plugin_id,
normalized_relative_path: script_path,
})
});
let decision = if routes_approval_to_guardian(parent_ctx) {
let review_cancel = cancel_token.child_token();
let review_rx = spawn_approval_request_review(
Arc::clone(parent_session),
Arc::clone(parent_ctx),
new_guardian_review_id(),
GuardianApprovalRequest::Shell {
id: call_id.clone(),
command,
cwd,
sandbox_permissions: if additional_permissions.is_some() {
crate::sandboxing::SandboxPermissions::WithAdditionalPermissions
} else {
crate::sandboxing::SandboxPermissions::UseDefault
},
additional_permissions,
justification: None,
},
reason,
GuardianReviewOptions {
plugin_attribution_override: plugin_attribution.clone(),
approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent,
external_cancel: Some(review_cancel.clone()),
},
);
await_approval_with_cancel(
receive_approval_review(review_rx),
parent_session,
&approval_id_for_op,
cancel_token,
Some(&review_cancel),
)
.await
} else {
await_approval_with_cancel(
parent_session.request_command_approval(
parent_ctx,
call_id,
approval_id,
environment_id,
command,
cwd,
reason,
network_approval_context,
proposed_execpolicy_amendment,
additional_permissions,
available_decisions,
plugin_attribution,
),
parent_session,
&approval_id_for_op,
cancel_token,
/*review_cancel_token*/ None,
)
.await
};
let _ = io
.submit(Op::ExecApproval {
id: approval_id_for_op,
turn_id: Some(turn_id),
decision,
})
.await;
}
/// Handle an ApplyPatchApprovalRequest by consulting the parent session and replying.
async fn handle_patch_approval(
io: &SessionIo,
_id: String,
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
event: ApplyPatchApprovalRequestEvent,
cancel_token: &CancellationToken,
) {
let ApplyPatchApprovalRequestEvent {
call_id,
changes,
reason,
grant_root,
..
} = event;
let approval_id = call_id.clone();
let guardian_decision = if routes_approval_to_guardian(parent_ctx) {
let files = changes
.keys()
.map(|path| {
#[allow(deprecated)]
parent_ctx.cwd.join(path)
})
.collect::<Vec<_>>();
let review_cancel = cancel_token.child_token();
let patch = changes
.iter()
.map(|(path, change)| match change {
codex_protocol::protocol::FileChange::Add { content } => {
format!("*** Add File: {}\n{}", path.display(), content)
}
codex_protocol::protocol::FileChange::Delete { content } => {
format!("*** Delete File: {}\n{}", path.display(), content)
}
codex_protocol::protocol::FileChange::Update {
unified_diff,
move_path,
} => {
if let Some(move_path) = move_path {
format!(
"*** Update File: {}\n*** Move to: {}\n{}",
path.display(),
move_path.display(),
unified_diff
)
} else {
format!("*** Update File: {}\n{}", path.display(), unified_diff)
}
}
})
.collect::<Vec<_>>()
.join("\n");
let review_rx = spawn_approval_request_review(
Arc::clone(parent_session),
Arc::clone(parent_ctx),
new_guardian_review_id(),
GuardianApprovalRequest::ApplyPatch {
id: approval_id.clone(),
#[allow(deprecated)]
cwd: parent_ctx.cwd.clone(),
files,
patch,
},
reason.clone(),
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent,
external_cancel: Some(review_cancel.clone()),
},
);
Some(
await_approval_with_cancel(
receive_approval_review(review_rx),
parent_session,
&approval_id,
cancel_token,
Some(&review_cancel),
)
.await,
)
} else {
None
};
let decision = if let Some(decision) = guardian_decision {
decision
} else {
let decision =
parent_session.request_patch_approval(parent_ctx, call_id, changes, reason, grant_root);
await_approval_with_cancel(
decision,
parent_session,
&approval_id,
cancel_token,
/*review_cancel_token*/ None,
)
.await
};
let _ = io
.submit(Op::PatchApproval {
id: approval_id,
decision,
})
.await;
}
async fn handle_request_user_input(
io: &SessionIo,
id: String,
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
pending_mcp_invocations: &Arc<Mutex<HashMap<String, PendingMcpInvocation>>>,
event: RequestUserInputEvent,
cancel_token: &CancellationToken,
) {
if let Some(response) = maybe_auto_review_mcp_request_user_input(
parent_session,
parent_ctx,
pending_mcp_invocations,
&event,
cancel_token,
)
.await
{
let _ = io.submit(Op::UserInputAnswer { id, response }).await;
return;
}
let args = RequestUserInputArgs {
questions: event.questions,
is_blocking: event.is_blocking,
auto_resolution_ms: event.auto_resolution_ms,
};
let response_fut =
parent_session.request_user_input(parent_ctx, parent_ctx.sub_id.clone(), args);
let response = await_user_input_with_cancel(
response_fut,
parent_session,
&parent_ctx.sub_id,
cancel_token,
)
.await;
let _ = io.submit(Op::UserInputAnswer { id, response }).await;
}
/// Intercepts delegated legacy MCP approval prompts on the RequestUserInput
/// compatibility path and, when guardian is active, answers them
/// programmatically after running the guardian review.
///
/// The RequestUserInput event only carries `call_id` plus approval question
/// metadata, so this helper joins it back to the child runtime metadata cached at
/// `McpToolCallBegin` in order to rebuild the full guardian review request.
async fn maybe_auto_review_mcp_request_user_input(
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
pending_mcp_invocations: &Arc<Mutex<HashMap<String, PendingMcpInvocation>>>,
event: &RequestUserInputEvent,
cancel_token: &CancellationToken,
) -> Option<RequestUserInputResponse> {
// TODO(ccunningham): Support delegated MCP approval elicitations here too after
// coordinating with @fouad. Today guardian only auto-reviews the RequestUserInput
// compatibility path for delegated MCP approvals.
let question = event
.questions
.iter()
.find(|question| is_mcp_tool_approval_question_id(&question.id))?;
let pending = pending_mcp_invocations
.lock()
.await
.get(&event.call_id)
.cloned()?;
let invocation = pending.invocation;
let metadata = pending.metadata;
let approvals_reviewer =
mcp_approvals_reviewer(parent_ctx, &invocation.server, metadata.as_ref());
if !routes_approval_to_guardian_with_reviewer(parent_ctx, approvals_reviewer) {
return None;
}
let review_cancel = cancel_token.child_token();
let review_rx = spawn_approval_request_review(
Arc::clone(parent_session),
Arc::clone(parent_ctx),
new_guardian_review_id(),
build_guardian_mcp_tool_review_request(&event.call_id, &invocation, metadata.as_ref()),
/*retry_reason*/ None,
GuardianReviewOptions {
plugin_attribution_override: None,
approval_request_source: GuardianApprovalRequestSource::DelegatedSubagent,
external_cancel: Some(review_cancel.clone()),
},
);
let decision = await_approval_with_cancel(
receive_approval_review(review_rx),
parent_session,
&event.call_id,
cancel_token,
Some(&review_cancel),
)
.await;
let selected_label = match decision {
ReviewDecision::ApprovedForSession => question
.options
.as_ref()
.and_then(|options| {
options
.iter()
.find(|option| option.label == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION)
})
.map(|option| option.label.clone())
.unwrap_or_else(|| MCP_TOOL_APPROVAL_ACCEPT.to_string()),
ReviewDecision::Approved
| ReviewDecision::ApprovedMcpPolicyAmendment
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
| ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(),
ReviewDecision::Denied { .. } | ReviewDecision::TimedOut | ReviewDecision::Abort => {
MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()
}
};
Some(RequestUserInputResponse {
answers: HashMap::from([(
question.id.clone(),
codex_protocol::request_user_input::RequestUserInputAnswer {
answers: vec![selected_label],
},
)]),
})
}
async fn handle_request_permissions(
io: &SessionIo,
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
event: RequestPermissionsEvent,
cancel_token: &CancellationToken,
) {
let call_id = event.call_id;
let args = RequestPermissionsArgs {
environment_id: event.environment_id,
reason: event.reason,
permissions: event.permissions,
};
let cwd = event.cwd.unwrap_or_else(|| {
#[allow(deprecated)]
parent_ctx.cwd.clone()
});
let response_fut = parent_session.request_permissions_for_cwd(
parent_ctx,
call_id.clone(),
args,
cwd,
cancel_token.clone(),
);
let response =
await_request_permissions_with_cancel(response_fut, parent_session, &call_id, cancel_token)
.await;
let _ = io
.submit(Op::RequestPermissionsResponse {
id: call_id,
response,
})
.await;
}
async fn await_user_input_with_cancel<F>(
fut: F,
parent_session: &Session,
sub_id: &str,
cancel_token: &CancellationToken,
) -> RequestUserInputResponse
where
F: core::future::Future<Output = Option<RequestUserInputResponse>>,
{
tokio::select! {
biased;
_ = cancel_token.cancelled() => {
let empty = RequestUserInputResponse {
answers: HashMap::new(),
};
parent_session
.notify_user_input_response(sub_id, empty.clone())
.await;
empty
}
response = fut => response.unwrap_or_else(|| RequestUserInputResponse {
answers: HashMap::new(),
}),
}
}
async fn await_request_permissions_with_cancel<F>(
fut: F,
parent_session: &Session,
call_id: &str,
cancel_token: &CancellationToken,
) -> RequestPermissionsResponse
where
F: core::future::Future<Output = Option<RequestPermissionsResponse>>,
{
tokio::select! {
biased;
_ = cancel_token.cancelled() => {
let empty = RequestPermissionsResponse {
permissions: Default::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
};
parent_session
.notify_request_permissions_response(call_id, empty.clone())
.await;
empty
}
response = fut => response.unwrap_or_else(|| RequestPermissionsResponse {
permissions: Default::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
}),
}
}
async fn receive_approval_review(review_rx: oneshot::Receiver<ReviewDecision>) -> ReviewDecision {
review_rx
.await
.unwrap_or_else(|_| ReviewDecision::denied("automatic approval review could not complete"))
}
/// Await an approval decision, aborting on cancellation.
async fn await_approval_with_cancel<F>(
fut: F,
parent_session: &Session,
approval_id: &str,
cancel_token: &CancellationToken,
review_cancel_token: Option<&CancellationToken>,
) -> codex_protocol::protocol::ReviewDecision
where
F: core::future::Future<Output = codex_protocol::protocol::ReviewDecision>,
{
tokio::select! {
biased;
_ = cancel_token.cancelled() => {
if let Some(review_cancel_token) = review_cancel_token {
review_cancel_token.cancel();
}
parent_session
.notify_approval(
approval_id,
codex_protocol::protocol::ReviewDecision::Abort,
)
.await;
codex_protocol::protocol::ReviewDecision::Abort
}
decision = fut => {
decision
}
}
}
#[cfg(test)]
#[path = "codex_delegate_tests.rs"]
mod tests;
codex-rs/core/src/codex_delegate_tests.rs
+28 -457
View File
@@ -1,61 +1,25 @@
use super::*;
use crate::environment_selection::TurnEnvironmentState;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX;
use async_channel::bounded;
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::error::CodexErrorDetails;
use codex_protocol::models::NetworkPermissions;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::ExecApprovalRequestEvent;
use codex_protocol::protocol::GuardianAssessmentAction;
use codex_protocol::protocol::GuardianAssessmentStatus;
use codex_protocol::protocol::GuardianCommandSource;
use codex_protocol::protocol::McpInvocation;
use codex_protocol::protocol::McpStartupCompleteEvent;
use codex_protocol::protocol::McpStartupStatus;
use codex_protocol::protocol::McpStartupUpdateEvent;
use codex_protocol::protocol::RawResponseItemEvent;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::TurnAbortReason;
use codex_protocol::protocol::TurnAbortedEvent;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsEvent;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputAnswer;
use codex_protocol::request_user_input::RequestUserInputEvent;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use core_test_support::PathBufExt;
use core_test_support::test_path_buf;
use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::Mutex;
use tokio::sync::oneshot;
use tokio::sync::watch;
use tokio::time::timeout;
#[tokio::test]
async fn dropped_approval_review_fails_closed() {
let (tx, rx) = oneshot::channel();
drop(tx);
assert_eq!(
receive_approval_review(rx).await,
ReviewDecision::denied("automatic approval review could not complete")
);
}
#[tokio::test]
async fn forward_events_filters_private_events_before_blocked_send_is_cancelled() {
let (tx_events, rx_events) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let (session, ctx, _rx_evt) = crate::session::tests::make_session_and_context_with_rx().await;
let io = Arc::new(SessionIo {
tx_sub,
rx_event: rx_events,
@@ -81,11 +45,7 @@ async fn forward_events_filters_private_events_before_blocked_send_is_cancelled(
let cancel = CancellationToken::new();
let forward = tokio::spawn(forward_events(
Arc::clone(&io),
Arc::clone(&session),
tx_out.clone(),
session,
ctx,
Arc::new(Mutex::new(HashMap::new())),
cancel.clone(),
));
@@ -207,6 +167,8 @@ async fn forward_ops_preserves_submission_trace_context() {
async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() {
let (parent_session, parent_ctx, _rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
let mut config = parent_ctx.config.as_ref().clone();
config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
let cancel_token = CancellationToken::new();
cancel_token.cancel();
let parent_environments = parent_ctx.environments.clone();
@@ -214,7 +176,7 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() {
let result = timeout(
Duration::from_secs(/*secs*/ 1),
run_codex_thread_interactive(
parent_ctx.config.as_ref().clone(),
config,
Arc::clone(&parent_session.services.auth_manager),
Arc::clone(&parent_session.services.models_manager),
parent_session,
@@ -237,426 +199,35 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() {
}
#[tokio::test]
async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
let (parent_session, mut parent_ctx, rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
*parent_session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let parent_ctx_mut = Arc::get_mut(&mut parent_ctx).expect("single turn context ref");
let TurnEnvironmentState::Ready(environment) = &mut parent_ctx_mut.environments.environments[0]
else {
panic!("expected ready primary environment");
};
environment.environment_id = "remote".to_string();
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let io = Arc::new(SessionIo {
tx_sub,
rx_event: rx_events_child,
agent_status,
session_loop_termination: completed_session_loop_termination(),
});
let call_id = "tool-call-1".to_string();
let expected_response = RequestPermissionsResponse {
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
},
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
};
#[allow(deprecated)]
let delegated_cwd = parent_ctx.cwd.join("delegated-cwd");
let cancel_token = CancellationToken::new();
let request_call_id = call_id.clone();
let request_cwd = delegated_cwd.clone();
let handle = tokio::spawn({
let io = Arc::clone(&io);
let parent_session = Arc::clone(&parent_session);
let parent_ctx = Arc::clone(&parent_ctx);
let cancel_token = cancel_token.clone();
async move {
handle_request_permissions(
io.as_ref(),
&parent_session,
&parent_ctx,
RequestPermissionsEvent {
call_id: request_call_id,
turn_id: "child-turn-1".to_string(),
environment_id: Some("remote".to_string()),
started_at_ms: 0,
reason: Some("need access".to_string()),
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
},
cwd: Some(request_cwd),
},
&cancel_token,
)
.await;
}
});
let request_event = timeout(Duration::from_secs(1), rx_events.recv())
.await
.expect("request_permissions event timed out")
.expect("request_permissions event missing");
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected RequestPermissions event");
};
assert_eq!(request.call_id, call_id.clone());
assert_eq!(request.environment_id.as_deref(), Some("remote"));
assert_eq!(request.cwd, Some(delegated_cwd));
parent_session
.notify_request_permissions_response(&call_id, expected_response.clone())
.await;
timeout(Duration::from_secs(1), handle)
.await
.expect("handle_request_permissions hung")
.expect("handle_request_permissions join error");
let submission = timeout(Duration::from_secs(1), rx_sub.recv())
.await
.expect("request_permissions response timed out")
.expect("request_permissions response missing");
let Op::RequestPermissionsResponse { id, response } = submission.op else {
panic!("expected request permissions response");
};
assert_eq!(id, call_id);
assert_eq!(response, expected_response);
}
#[tokio::test]
async fn handle_request_user_input_preserves_non_blocking_flag_for_round_trip() {
let (parent_session, parent_ctx, rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
*parent_session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let io = Arc::new(SessionIo {
tx_sub,
rx_event: rx_events_child,
agent_status,
session_loop_termination: completed_session_loop_termination(),
});
let pending_mcp_invocations = Arc::new(Mutex::new(HashMap::new()));
let cancel_token = CancellationToken::new();
let child_event_id = "child-request-1".to_string();
let parent_user_input_id = parent_ctx.sub_id.clone();
let expected_response = RequestUserInputResponse {
answers: HashMap::from([(
"q1".to_string(),
RequestUserInputAnswer {
answers: vec!["yes".to_string()],
},
)]),
};
let handle = tokio::spawn({
let io = Arc::clone(&io);
let parent_session = Arc::clone(&parent_session);
let parent_ctx = Arc::clone(&parent_ctx);
let pending_mcp_invocations = Arc::clone(&pending_mcp_invocations);
let cancel_token = cancel_token.clone();
let child_event_id = child_event_id.clone();
async move {
handle_request_user_input(
io.as_ref(),
child_event_id,
&parent_session,
&parent_ctx,
&pending_mcp_invocations,
RequestUserInputEvent {
call_id: "child-call-1".to_string(),
turn_id: "child-turn-1".to_string(),
questions: vec![RequestUserInputQuestion {
id: "q1".to_string(),
header: "Confirm".to_string(),
question: "Continue?".to_string(),
is_other: false,
is_secret: false,
options: None,
}],
is_blocking: false,
auto_resolution_ms: None,
},
&cancel_token,
)
.await;
}
});
let request_event = timeout(Duration::from_secs(1), rx_events.recv())
.await
.expect("request_user_input event timed out")
.expect("request_user_input event missing");
let EventMsg::RequestUserInput(request) = request_event.msg else {
panic!("expected RequestUserInput event");
};
assert_eq!(request.call_id, parent_user_input_id);
assert_eq!(request.is_blocking, false);
parent_session
.notify_user_input_response(&parent_user_input_id, expected_response.clone())
.await;
timeout(Duration::from_secs(1), handle)
.await
.expect("handle_request_user_input hung")
.expect("handle_request_user_input join error");
let submission = timeout(Duration::from_secs(1), rx_sub.recv())
.await
.expect("request_user_input response timed out")
.expect("request_user_input response missing");
let Op::UserInputAnswer { id, response } = submission.op else {
panic!("expected user input answer");
};
assert_eq!(id, child_event_id);
assert_eq!(response, expected_response);
}
#[tokio::test]
async fn handle_exec_approval_uses_call_id_for_guardian_review_and_approval_id_for_reply() {
let (parent_session, parent_ctx, rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
let mut parent_ctx = Arc::try_unwrap(parent_ctx).expect("single turn context ref");
let mut config = (*parent_ctx.config).clone();
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
parent_ctx.config = Arc::new(config);
Arc::make_mut(&mut parent_ctx.config)
.permissions
.approval_policy
.set(AskForApproval::OnRequest)
.expect("set on-request policy");
let parent_ctx = Arc::new(parent_ctx);
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let io = Arc::new(SessionIo {
tx_sub,
rx_event: rx_events_child,
agent_status,
session_loop_termination: completed_session_loop_termination(),
});
let cancel_token = CancellationToken::new();
let handle = tokio::spawn({
let io = Arc::clone(&io);
let parent_session = Arc::clone(&parent_session);
let parent_ctx = Arc::clone(&parent_ctx);
let cancel_token = cancel_token.clone();
async move {
handle_exec_approval(
io.as_ref(),
"child-turn-1".to_string(),
&parent_session,
&parent_ctx,
ExecApprovalRequestEvent {
call_id: "command-item-1".to_string(),
plugin_id: Some("sample@openai-curated".to_string()),
script_path: Some("scripts/run.py".to_string()),
approval_id: Some("callback-approval-1".to_string()),
turn_id: "child-turn-1".to_string(),
environment_id: Some("remote".to_string()),
started_at_ms: 0,
command: vec!["rm".to_string(), "-rf".to_string(), "tmp".to_string()],
cwd: test_path_buf("/tmp").abs(),
reason: Some("unsafe subcommand".to_string()),
network_approval_context: None,
proposed_execpolicy_amendment: None,
proposed_network_policy_amendments: None,
additional_permissions: None,
available_decisions: Some(vec![
ReviewDecision::Approved,
ReviewDecision::Abort,
]),
parsed_cmd: Vec::new(),
},
&cancel_token,
)
.await;
}
});
let assessment_event = timeout(Duration::from_secs(2), async {
loop {
let event = rx_events.recv().await.expect("guardian assessment event");
if let EventMsg::GuardianAssessment(assessment) = event.msg {
return assessment;
}
}
})
.await
.expect("timed out waiting for guardian assessment");
let expected_action = GuardianAssessmentAction::Command {
source: GuardianCommandSource::Shell,
command: "rm -rf tmp".to_string(),
cwd: test_path_buf("/tmp").abs(),
};
assert!(!assessment_event.id.is_empty());
assert_eq!(
assessment_event.target_item_id.as_deref(),
Some("command-item-1")
);
assert_eq!(
assessment_event.plugin_id.as_deref(),
Some("sample@openai-curated")
);
assert_eq!(
assessment_event.script_path.as_deref(),
Some("scripts/run.py")
);
assert_eq!(assessment_event.turn_id, parent_ctx.sub_id);
assert_eq!(
assessment_event.status,
GuardianAssessmentStatus::InProgress
);
assert_eq!(assessment_event.risk_level, None);
assert_eq!(assessment_event.user_authorization, None);
assert_eq!(assessment_event.rationale, None);
assert_eq!(assessment_event.decision_source, None);
assert_eq!(assessment_event.action, expected_action);
cancel_token.cancel();
timeout(Duration::from_secs(2), handle)
.await
.expect("handle_exec_approval hung")
.expect("handle_exec_approval join error");
let submission = timeout(Duration::from_secs(2), rx_sub.recv())
.await
.expect("exec approval response timed out")
.expect("exec approval response missing");
let Op::ExecApproval {
id,
turn_id,
decision,
} = submission.op
else {
panic!("expected exec approval");
};
assert_eq!(id, "callback-approval-1");
assert_eq!(turn_id, Some("child-turn-1".to_string()));
assert_eq!(decision, ReviewDecision::Abort);
}
#[tokio::test]
async fn delegated_mcp_guardian_abort_returns_synthetic_decline_answer() {
async fn run_codex_thread_interactive_rejects_approval_policy_that_can_prompt() {
let (parent_session, parent_ctx, _rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
let mut parent_ctx = Arc::try_unwrap(parent_ctx).expect("single turn context ref");
let mut config = (*parent_ctx.config).clone();
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
parent_ctx.config = Arc::new(config);
Arc::make_mut(&mut parent_ctx.config)
.permissions
.approval_policy
.set(AskForApproval::OnRequest)
.expect("set on-request policy");
let parent_ctx = Arc::new(parent_ctx);
let mut config = parent_ctx.config.as_ref().clone();
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
let parent_environments = parent_ctx.environments.clone();
let pending_mcp_invocations = Arc::new(Mutex::new(HashMap::from([(
"call-1".to_string(),
PendingMcpInvocation {
invocation: McpInvocation {
server: "custom_server".to_string(),
tool: "dangerous_tool".to_string(),
arguments: None,
},
metadata: None,
},
)])));
let cancel_token = CancellationToken::new();
cancel_token.cancel();
let response = maybe_auto_review_mcp_request_user_input(
&parent_session,
&parent_ctx,
&pending_mcp_invocations,
&RequestUserInputEvent {
call_id: "call-1".to_string(),
turn_id: "child-turn-1".to_string(),
questions: vec![RequestUserInputQuestion {
id: format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_call-1"),
header: "Approve app tool call?".to_string(),
question: "Allow this app tool?".to_string(),
is_other: false,
is_secret: false,
options: None,
}],
is_blocking: true,
auto_resolution_ms: None,
},
&cancel_token,
let result = run_codex_thread_interactive(
config,
Arc::clone(&parent_session.services.auth_manager),
Arc::clone(&parent_session.services.models_manager),
parent_session,
parent_ctx,
parent_environments,
CancellationToken::new(),
SubAgentSource::Review,
/*initial_history*/ None,
crate::session::GitEnrichmentPolicy::Fresh,
codex_sandboxing::WindowsSandboxProxySettingsMode::Reconcile,
)
.await;
assert_eq!(
response,
Some(RequestUserInputResponse {
answers: HashMap::from([(
format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_call-1"),
RequestUserInputAnswer {
answers: vec![MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()],
},
)]),
})
);
}
#[tokio::test]
async fn delegated_mcp_user_reviewer_returns_none_without_metadata() {
let (parent_session, parent_ctx, _rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
let pending_mcp_invocations = Arc::new(Mutex::new(HashMap::from([(
"call-1".to_string(),
PendingMcpInvocation {
invocation: McpInvocation {
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
tool: "dangerous_tool".to_string(),
arguments: None,
},
metadata: None,
},
)])));
let cancel_token = CancellationToken::new();
let event = RequestUserInputEvent {
call_id: "call-1".to_string(),
turn_id: "child-turn-1".to_string(),
questions: vec![RequestUserInputQuestion {
id: format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_call-1"),
header: "Approve app tool call?".to_string(),
question: "Allow this app tool?".to_string(),
is_other: false,
is_secret: false,
options: None,
}],
is_blocking: true,
auto_resolution_ms: None,
};
let response = maybe_auto_review_mcp_request_user_input(
&parent_session,
&parent_ctx,
&pending_mcp_invocations,
&event,
&cancel_token,
)
.await;
assert_eq!(response, None);
assert!(matches!(
result,
Err(err)
if matches!(
err.details(),
CodexErrorDetails::InvalidRequest(message)
if message == "Codex delegates require approval policy `never`"
)
));
}
codex-rs/core/src/connectors.rs
-15
View File
@@ -504,21 +504,6 @@ pub fn with_app_plugin_sources(
connectors
}
pub(crate) fn mcp_approvals_reviewer(
config: &Config,
model: Option<&str>,
server_name: &str,
connector_id: Option<&str>,
) -> ApprovalsReviewer {
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
model,
server_name,
connector_id,
)
}
pub(crate) fn mcp_approvals_reviewer_from_layers(
config_layer_stack: &codex_config::ConfigLayerStack,
default_reviewer: ApprovalsReviewer,
codex-rs/core/src/connectors_tests.rs
+18 -12
View File
@@ -345,8 +345,9 @@ approvals_reviewer = "{app}"
.expect("config should build");
assert_eq!(
mcp_approvals_reviewer(
&config,
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("calendar")
@@ -354,8 +355,9 @@ approvals_reviewer = "{app}"
expected_app
);
assert_eq!(
mcp_approvals_reviewer(
&config,
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("drive")
@@ -363,8 +365,9 @@ approvals_reviewer = "{app}"
expected_default
);
assert_eq!(
mcp_approvals_reviewer(
&config,
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
/*connector_id*/ None
@@ -372,8 +375,9 @@ approvals_reviewer = "{app}"
expected_default
);
assert_eq!(
mcp_approvals_reviewer(
&config,
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
config.model.as_deref(),
"custom_server",
Some("calendar")
@@ -408,8 +412,9 @@ approvals_reviewer = "user"
.expect("config should build");
assert_eq!(
mcp_approvals_reviewer(
&config,
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("calendar")
@@ -443,8 +448,9 @@ approvals_reviewer = "user"
.expect("config should build");
assert_eq!(
mcp_approvals_reviewer(
&config,
mcp_approvals_reviewer_from_layers(
&config.config_layer_stack,
config.approvals_reviewer,
config.model.as_deref(),
CODEX_APPS_MCP_SERVER_NAME,
Some("calendar")
codex-rs/core/src/guardian/mod.rs
-1
View File
@@ -44,7 +44,6 @@ pub(crate) use review::review_approval_request;
pub(crate) use review::review_approval_request_with_cancel;
pub(crate) use review::routes_approval_policy_to_guardian;
pub(crate) use review::routes_approval_to_guardian;
pub(crate) use review::routes_approval_to_guardian_with_reviewer;
pub(crate) use review::spawn_approval_request_review;
pub(crate) use review_session::GuardianReviewSessionManager;
pub(crate) use review_session::prompt_cache_key_override_for_review_session;
codex-rs/core/src/guardian/tests.rs
+1
View File
@@ -12,6 +12,7 @@ use crate::guardian::prompt::BUNDLED_GUARDIAN_POLICY;
use crate::guardian::prompt::BUNDLED_GUARDIAN_POLICY_TEMPLATE;
use crate::guardian::prompt::guardian_policy_prompt_with_config_and_template;
use crate::guardian::review::guardian_review_session_config;
use crate::guardian::review::routes_approval_to_guardian_with_reviewer;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::session::turn_context::TurnEnvironment;
codex-rs/core/src/mcp_skill_dependencies.rs
+5
View File
@@ -7,6 +7,7 @@ use codex_config::McpServerTransportConfig;
use codex_config::load_global_mcp_servers;
use codex_login::default_client::is_first_party_originator;
use codex_login::default_client::originator;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::request_user_input::RequestUserInputArgs;
use codex_protocol::request_user_input::RequestUserInputQuestion;
use codex_protocol::request_user_input::RequestUserInputQuestionOption;
@@ -246,6 +247,10 @@ async fn should_install_mcp_dependencies(
return true;
}
if turn_context.approval_policy() == AskForApproval::Never {
return false;
}
let server_list = format_missing_mcp_dependencies(missing);
let question = RequestUserInputQuestion {
id: SKILL_MCP_DEPENDENCY_PROMPT_ID.to_string(),
codex-rs/core/src/mcp_tool_call.rs
+7 -30
View File
@@ -26,7 +26,6 @@ use codex_analytics::build_track_events_context;
use codex_api::HostedFileUploadContext;
use codex_config::ConfigLayerSource;
use codex_config::types::AppToolApproval;
use codex_config::types::ApprovalsReviewer;
use codex_connectors::AppToolPolicy;
use codex_connectors::AppToolPolicyEvaluator;
use codex_connectors::AppToolPolicyInput;
@@ -1264,20 +1263,9 @@ struct McpToolApprovalElicitationRequest<'a> {
pub(crate) const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval";
pub(crate) const MCP_TOOL_APPROVAL_ACCEPT: &str = "Allow";
pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this session";
// Internal-only token used when guardian auto-reviews delegated MCP approvals on the
// RequestUserInput compatibility path. That legacy MCP prompt has allow/cancel labels but no
// real "Decline" answer, so this lets guardian denials round-trip distinctly from user cancel.
// This is not a user-facing option.
pub(crate) const MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC: &str = "__codex_mcp_decline__";
const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Allow and don't ask me again";
const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel";
pub(crate) fn is_mcp_tool_approval_question_id(question_id: &str) -> bool {
question_id
.strip_prefix(MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX)
.is_some_and(|suffix| suffix.starts_with('_'))
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
struct McpToolApprovalKey {
server: String,
@@ -1407,6 +1395,7 @@ pub(crate) async fn request_mcp_tool_user_approval(
connected_account_email,
tool_title,
tool_description,
approval_policy,
approval_mode,
allow_session_remember,
allow_persistent_approval,
@@ -1416,6 +1405,12 @@ pub(crate) async fn request_mcp_tool_user_approval(
unreachable!("only MCP actions can request MCP tool approval");
};
if *approval_policy == AskForApproval::Never {
return ReviewDecision::denied(
"MCP tool call requires approval, but approval policy is never",
);
}
let tool_call_mcp_elicitation_enabled = turn_context
.config
.features
@@ -1501,19 +1496,6 @@ pub(crate) async fn request_mcp_tool_user_approval(
)
}
pub(crate) fn mcp_approvals_reviewer(
turn_context: &TurnContext,
server_name: &str,
metadata: Option<&McpToolApprovalMetadata>,
) -> ApprovalsReviewer {
connectors::mcp_approvals_reviewer(
turn_context.config.as_ref(),
Some(turn_context.model_info.slug.as_str()),
server_name,
metadata.and_then(|metadata| metadata.connector_id.as_deref()),
)
}
fn session_mcp_tool_approval_key(
invocation: &McpInvocation,
metadata: Option<&McpToolApprovalMetadata>,
@@ -1938,11 +1920,6 @@ fn parse_mcp_tool_approval_response(
return ReviewDecision::Abort;
};
if answers
.iter()
.any(|answer| answer == MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC)
{
ReviewDecision::denied("user rejected MCP tool call")
} else if answers
.iter()
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION)
{
codex-rs/core/src/mcp_tool_call_tests.rs
-20
View File
@@ -1822,26 +1822,6 @@ fn declined_elicitation_response_stays_decline() {
);
}
#[test]
fn synthetic_decline_request_user_input_response_stays_decline() {
let response = parse_mcp_tool_approval_response(
Some(RequestUserInputResponse {
answers: HashMap::from([(
"approval".to_string(),
RequestUserInputAnswer {
answers: vec![MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()],
},
)]),
}),
"approval",
);
assert_eq!(
response,
ReviewDecision::denied("user rejected MCP tool call")
);
}
#[test]
fn accepted_elicitation_response_uses_always_persist_meta() {
let response = parse_mcp_tool_approval_elicitation_response(
codex-rs/core/src/session/mod.rs
-34
View File
@@ -2667,40 +2667,6 @@ impl Session {
}
}
pub(crate) async fn request_permissions_for_cwd(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
let turn_environment = match args.environment_id.as_deref() {
Some(environment_id) => turn_context
.environments
.turn_environments()
.find(|environment| environment.environment_id == environment_id),
None => turn_context.environments.primary(),
};
let Some(turn_environment) = turn_environment else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
let mut environment = turn_environment.selection();
environment.cwd = PathUri::from_abs_path(&cwd);
self.request_permissions_for_environment(
turn_context,
call_id,
args,
environment,
cancellation_token,
)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
codex-rs/core/tests/suite/codex_delegate.rs
+411
View File
@@ -1,15 +1,34 @@
use codex_core::config::Constrained;
use codex_exec_server::CreateDirectoryOptions;
use codex_features::Feature;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewRequest;
use codex_protocol::protocol::ReviewTarget;
use codex_protocol::user_input::UserInput;
use codex_utils_path_uri::PathUri;
use core_test_support::PathExt;
use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL;
use core_test_support::apps_test_server::SEARCH_CALENDAR_NAMESPACE;
use core_test_support::apps_test_server::recorded_apps_tool_calls;
use core_test_support::apps_test_server::search_capable_apps_builder;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_function_call;
use core_test_support::responses::ev_function_call_with_namespace;
use core_test_support::responses::ev_reasoning_item_added;
use core_test_support::responses::ev_reasoning_summary_text_delta;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::skip_if_wine_exec;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
@@ -58,3 +77,395 @@ async fn codex_delegate_ignores_legacy_deltas() {
assert_eq!(reasoning_delta_count, 1, "expected one new reasoning delta");
}
#[cfg_attr(windows, ignore = "no exec_command on Windows")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn codex_delegate_rejects_escalation_requests_when_parent_can_prompt() {
skip_if_no_network!();
let call_id = "review-escalation-call";
let command = serde_json::json!({
"cmd": "echo review",
"sandbox_permissions": "require_escalated"
});
let server = start_mock_server().await;
let response_mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call(call_id, "exec_command", &command.to_string()),
ev_completed("resp-1"),
]),
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
],
)
.await;
let test = test_codex()
.with_config(|config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
})
.build_with_auto_env(&server)
.await
.expect("build review delegate with escalation support");
test.codex
.submit(Op::Review {
review_request: ReviewRequest {
target: ReviewTarget::Custom {
instructions: "Review without requesting approval".to_string(),
},
user_facing_hint: None,
},
})
.await
.expect("submit review");
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
assert!(
matches!(event, EventMsg::TurnComplete(_)),
"review delegate should reject escalation requests without prompting: {event:?}"
);
let requests = response_mock.requests();
let [_, completion_request] = requests.as_slice() else {
panic!("expected the model request and escalation-denial continuation");
};
let output = completion_request.function_call_output(call_id);
let response = output["output"]
.as_str()
.expect("command tool output should be a string");
assert!(
response.contains("approval policy is Never"),
"escalation should be rejected by the delegate's approval policy: {response}"
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn codex_delegate_rejects_legacy_mcp_approvals_without_prompting() {
skip_if_no_network!();
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server)
.await
.expect("mount mock app server");
let call_id = "review-calendar-call";
let arguments = serde_json::json!({
"title": "Review meeting",
"starts_at": "2026-08-12T12:00:00Z"
});
let response_mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-1"),
ev_function_call_with_namespace(
call_id,
SEARCH_CALENDAR_NAMESPACE,
SEARCH_CALENDAR_CREATE_TOOL,
&arguments.to_string(),
),
ev_completed("resp-1"),
]),
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
],
)
.await;
let test = search_capable_apps_builder(apps_server.chatgpt_base_url)
.with_config(|config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config
.permissions
.set_permission_profile(PermissionProfile::read_only())
.expect("review delegate should inherit a restricted permission profile");
let config_path = config.codex_home.join("config.toml").abs();
let app_config = toml::from_str(
r#"
[apps.calendar]
default_tools_approval_mode = "prompt"
"#,
)
.expect("app approval configuration should parse");
config.config_layer_stack = config
.config_layer_stack
.with_user_config(&config_path, app_config)
.expect("app approval configuration should be valid");
config
.features
.disable(Feature::ToolCallMcpElicitation)
.expect("legacy MCP approvals should be available");
})
.build_with_auto_env(&server)
.await
.expect("build review delegate with legacy MCP approvals");
test.codex
.submit(Op::Review {
review_request: ReviewRequest {
target: ReviewTarget::Custom {
instructions: "Review the [$calendar](app://calendar) integration".to_string(),
},
user_facing_hint: None,
},
})
.await
.expect("submit review");
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::RequestUserInput(_)
| EventMsg::ElicitationRequest(_)
| EventMsg::TurnComplete(_)
)
})
.await;
assert!(
matches!(event, EventMsg::TurnComplete(_)),
"review delegate should reject legacy MCP approvals without prompting: {event:?}"
);
let requests = response_mock.requests();
let [_, completion_request] = requests.as_slice() else {
panic!("expected the model request and MCP-denial continuation");
};
let output = completion_request.function_call_output(call_id);
let response = output["output"]
.as_str()
.expect("MCP tool output should be a string");
assert!(
response.contains("approval policy is never"),
"MCP tool should be rejected by the delegate's approval policy: {response}"
);
assert!(
recorded_apps_tool_calls(&server).await.is_empty(),
"MCP tool requiring approval should never execute"
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn codex_delegate_rejects_skill_mcp_dependency_installation_without_prompting() {
skip_if_wine_exec!("skill paths require matching host and executor path conventions");
skip_if_no_network!();
let server = start_mock_server().await;
let response_mock = mount_sse_once(
&server,
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
)
.await;
let test = test_codex()
.with_config(|config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config
.permissions
.set_permission_profile(PermissionProfile::read_only())
.expect("review delegate should inherit a restricted permission profile");
config
.features
.enable(Feature::SkillMcpDependencyInstall)
.expect("skill MCP dependency installation should be available");
})
.with_workspace_setup(|cwd, fs| async move {
let skill_dir = cwd.join(".agents/skills/dependency-skill");
let agents_dir = skill_dir.join("agents");
fs.create_directory(
&PathUri::from_host_native_path(&agents_dir)?,
CreateDirectoryOptions { recursive: true },
/*sandbox*/ None,
)
.await?;
fs.write_file(
&PathUri::from_host_native_path(skill_dir.join("SKILL.md"))?,
b"---\nname: dependency-skill\ndescription: Requires an MCP server.\n---\n\nReview dependency instructions.\n"
.to_vec(),
/*sandbox*/ None,
)
.await?;
fs.write_file(
&PathUri::from_host_native_path(agents_dir.join("openai.yaml"))?,
b"dependencies:\n tools:\n - type: mcp\n value: missing-review-server\n transport: streamable_http\n url: http://127.0.0.1:1/mcp\n"
.to_vec(),
/*sandbox*/ None,
)
.await?;
Ok(())
})
.build_with_auto_env(&server)
.await
.expect("build review delegate with a missing skill MCP dependency");
test.codex
.submit(Op::Review {
review_request: ReviewRequest {
target: ReviewTarget::Custom {
instructions: "Review with $dependency-skill".to_string(),
},
user_facing_hint: None,
},
})
.await
.expect("submit review");
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::RequestUserInput(_) | EventMsg::TurnComplete(_)
)
})
.await;
assert!(
matches!(event, EventMsg::TurnComplete(_)),
"review delegate should reject skill MCP dependency installation without prompting: {event:?}"
);
let request = response_mock.single_request();
let user_texts = request.message_input_texts("user");
assert!(
user_texts.iter().any(|text| {
text.contains("<skill>\n<name>dependency-skill</name>")
&& text.contains("Review dependency instructions.")
}),
"review should continue with the selected skill after rejecting its MCP dependency: {user_texts:?}"
);
}
#[cfg_attr(windows, ignore = "no exec_command on Windows")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn guardian_delegate_rejects_escalation_requests_without_prompting() {
skip_if_wine_exec!("Guardian approval actions require host-native paths");
skip_if_no_network!();
let server = start_mock_server().await;
let test = test_codex()
.with_config(|config| {
config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest);
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
config
.permissions
.set_permission_profile(PermissionProfile::read_only())
.expect("guardian delegate should inherit a restricted permission profile");
})
.build_with_auto_env(&server)
.await
.expect("build guardian delegate with escalation support");
let parent_call_id = "parent-escalation-call";
let guardian_call_id = "guardian-escalation-call";
let guardian_output_file = test.cwd.path().join("guardian-escalation-marker.txt");
let parent_command = serde_json::json!({
"cmd": "echo parent command",
"sandbox_permissions": "require_escalated",
"justification": "Trigger Guardian approval review."
});
let guardian_command = serde_json::json!({
"cmd": format!("echo guardian-ran > \"{}\"", guardian_output_file.display()),
"sandbox_permissions": "require_escalated",
"justification": "Guardian must not escalate its own commands."
});
let assessment = serde_json::json!({
"risk_level": "high",
"user_authorization": "low",
"outcome": "deny",
"rationale": "Guardian could not execute an escalated command."
});
let response_mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("resp-parent-command"),
ev_function_call(parent_call_id, "exec_command", &parent_command.to_string()),
ev_completed("resp-parent-command"),
]),
sse(vec![
ev_response_created("resp-guardian-command"),
ev_function_call(
guardian_call_id,
"exec_command",
&guardian_command.to_string(),
),
ev_completed("resp-guardian-command"),
]),
sse(vec![
ev_response_created("resp-guardian-assessment"),
ev_assistant_message("msg-guardian-assessment", &assessment.to_string()),
ev_completed("resp-guardian-assessment"),
]),
sse(vec![
ev_response_created("resp-parent-denied"),
ev_assistant_message("msg-parent-denied", "denied"),
ev_completed("resp-parent-denied"),
]),
],
)
.await;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "Trigger Guardian review of an escalated command".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
responsesapi_client_metadata: None,
additional_context: Default::default(),
thread_settings: codex_protocol::protocol::ThreadSettingsOverrides {
approval_policy: Some(AskForApproval::OnRequest),
approvals_reviewer: Some(ApprovalsReviewer::AutoReview),
..Default::default()
},
})
.await
.expect("submit guardian-reviewed command");
let event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
assert!(
matches!(event, EventMsg::TurnComplete(_)),
"guardian delegate should reject escalation requests without prompting: {event:?}"
);
let requests = response_mock.requests();
let guardian_requests = requests
.iter()
.filter(|request| {
request.body_json()["client_metadata"]["x-openai-subagent"].as_str() == Some("guardian")
})
.collect::<Vec<_>>();
assert_eq!(guardian_requests.len(), 2);
let guardian_output = guardian_requests
.iter()
.find_map(|request| request.function_call_output_text(guardian_call_id))
.expect("guardian continuation should include the rejected command output");
assert!(
guardian_output.contains("approval policy is Never"),
"guardian escalation should be rejected by its never approval policy: {guardian_output}"
);
assert!(
!guardian_output_file.exists(),
"guardian command requiring approval should never execute"
);
let parent_output = requests
.iter()
.find_map(|request| request.function_call_output_text(parent_call_id))
.expect("parent continuation should include the guardian denial");
assert!(
parent_output.contains("Guardian could not execute an escalated command."),
"guardian denial rationale should reach the parent: {parent_output}"
);
}