mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Route opted-in MCP elicitations through Guardian (#19431)
# Motivation Browser Use origin-access prompts are MCP elicitations, not direct tool-call approval prompts, so they were bypassing the Guardian approval path. We need a generic opt-in that lets eligible MCP elicitations use Guardian when the current turn already routes approvals there. # Description Add a generic elicitation reviewer hook in codex-mcp and wire codex-core to pass a Guardian reviewer callback when creating the MCP connection manager. The reviewer validates explicit mcp_tool_call opt-in metadata, builds a Guardian MCP tool-call review request from server/tool/connector metadata and tool params, and maps Guardian approval, denial, timeout, and cancellation decisions back to MCP elicitation responses. The new option to trigger this in the `_meta` object is: ``` "codex_request_type": "approval_request", ``` # Testing - RUST_MIN_STACK=8388608 NEXTEST_STATUS_LEVEL=leak cargo nextest run --no-fail-fast --cargo-profile ci-test --test-threads 2 - cargo clippy --tests -- -D warnings - cargo fmt -- --config imports_granularity=Item --check - cargo shear - pnpm run format - python3 .github/scripts/verify_cargo_workspace_manifests.py - python3 .github/scripts/verify_tui_core_boundary.py - python3 .github/scripts/verify_bazel_clippy_lints.py - git diff --check
This commit is contained in:
@@ -17,6 +17,7 @@ use crate::codex_apps::CodexAppsToolsCacheContext;
|
||||
use crate::codex_apps::CodexAppsToolsCacheKey;
|
||||
use crate::codex_apps::write_cached_codex_apps_tools_if_needed;
|
||||
use crate::elicitation::ElicitationRequestManager;
|
||||
use crate::elicitation::ElicitationReviewerHandle;
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::mcp::ToolPluginProvenance;
|
||||
use crate::rmcp_client::AsyncManagedClient;
|
||||
@@ -87,6 +88,7 @@ impl McpConnectionManager {
|
||||
elicitation_requests: ElicitationRequestManager::new(
|
||||
approval_policy.value(),
|
||||
permission_profile.get().clone(),
|
||||
/*reviewer*/ None,
|
||||
),
|
||||
startup_cancellation_token: CancellationToken::new(),
|
||||
}
|
||||
@@ -157,13 +159,17 @@ impl McpConnectionManager {
|
||||
host_owned_codex_apps_enabled: bool,
|
||||
tool_plugin_provenance: ToolPluginProvenance,
|
||||
auth: Option<&CodexAuth>,
|
||||
elicitation_reviewer: Option<ElicitationReviewerHandle>,
|
||||
) -> (Self, CancellationToken) {
|
||||
let cancel_token = CancellationToken::new();
|
||||
let mut clients = HashMap::new();
|
||||
let mut server_origins = HashMap::new();
|
||||
let mut join_set = JoinSet::new();
|
||||
let elicitation_requests =
|
||||
ElicitationRequestManager::new(approval_policy.value(), initial_permission_profile);
|
||||
let elicitation_requests = ElicitationRequestManager::new(
|
||||
approval_policy.value(),
|
||||
initial_permission_profile,
|
||||
elicitation_reviewer,
|
||||
);
|
||||
let tool_plugin_provenance = Arc::new(tool_plugin_provenance);
|
||||
let startup_submit_id = submit_id.clone();
|
||||
let codex_apps_auth_provider = auth
|
||||
|
||||
@@ -203,8 +203,11 @@ fn elicitation_granular_policy_respects_never_and_config() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn disabled_permissions_auto_accept_elicitation_with_empty_form_schema() {
|
||||
let manager =
|
||||
ElicitationRequestManager::new(AskForApproval::Never, PermissionProfile::Disabled);
|
||||
let manager = ElicitationRequestManager::new(
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::Disabled,
|
||||
/*reviewer*/ None,
|
||||
);
|
||||
let (tx_event, _rx_event) = async_channel::bounded(1);
|
||||
let sender = manager.make_sender("server".to_string(), tx_event);
|
||||
|
||||
@@ -233,8 +236,11 @@ async fn disabled_permissions_auto_accept_elicitation_with_empty_form_schema() {
|
||||
|
||||
#[tokio::test]
|
||||
async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fields() {
|
||||
let manager =
|
||||
ElicitationRequestManager::new(AskForApproval::Never, PermissionProfile::Disabled);
|
||||
let manager = ElicitationRequestManager::new(
|
||||
AskForApproval::Never,
|
||||
PermissionProfile::Disabled,
|
||||
/*reviewer*/ None,
|
||||
);
|
||||
let (tx_event, _rx_event) = async_channel::bounded(1);
|
||||
let sender = manager.make_sender("server".to_string(), tx_event);
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_rmcp_client::ElicitationResponse;
|
||||
use codex_rmcp_client::SendElicitation;
|
||||
use futures::future::BoxFuture;
|
||||
use futures::future::FutureExt;
|
||||
use rmcp::model::CreateElicitationRequestParams;
|
||||
use rmcp::model::ElicitationAction;
|
||||
@@ -31,24 +32,43 @@ use rmcp::model::RequestId;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ElicitationReviewRequest {
|
||||
pub server_name: String,
|
||||
pub request_id: RequestId,
|
||||
pub elicitation: CreateElicitationRequestParams,
|
||||
}
|
||||
|
||||
pub trait ElicitationReviewer: Send + Sync {
|
||||
fn review(
|
||||
&self,
|
||||
request: ElicitationReviewRequest,
|
||||
) -> BoxFuture<'static, Result<Option<ElicitationResponse>>>;
|
||||
}
|
||||
|
||||
pub type ElicitationReviewerHandle = Arc<dyn ElicitationReviewer>;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ElicitationRequestManager {
|
||||
requests: Arc<Mutex<ResponderMap>>,
|
||||
pub(crate) approval_policy: Arc<StdMutex<AskForApproval>>,
|
||||
pub(crate) permission_profile: Arc<StdMutex<PermissionProfile>>,
|
||||
auto_deny: Arc<StdMutex<bool>>,
|
||||
reviewer: Option<ElicitationReviewerHandle>,
|
||||
}
|
||||
|
||||
impl ElicitationRequestManager {
|
||||
pub(crate) fn new(
|
||||
approval_policy: AskForApproval,
|
||||
permission_profile: PermissionProfile,
|
||||
reviewer: Option<ElicitationReviewerHandle>,
|
||||
) -> Self {
|
||||
Self {
|
||||
requests: Arc::new(Mutex::new(HashMap::new())),
|
||||
approval_policy: Arc::new(StdMutex::new(approval_policy)),
|
||||
permission_profile: Arc::new(StdMutex::new(permission_profile)),
|
||||
auto_deny: Arc::new(StdMutex::new(false)),
|
||||
reviewer,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -89,6 +109,7 @@ impl ElicitationRequestManager {
|
||||
let approval_policy = self.approval_policy.clone();
|
||||
let permission_profile = self.permission_profile.clone();
|
||||
let auto_deny = self.auto_deny.clone();
|
||||
let reviewer = self.reviewer.clone();
|
||||
Box::new(move |id, elicitation| {
|
||||
let elicitation_requests = elicitation_requests.clone();
|
||||
let tx_event = tx_event.clone();
|
||||
@@ -96,6 +117,7 @@ impl ElicitationRequestManager {
|
||||
let approval_policy = approval_policy.clone();
|
||||
let permission_profile = permission_profile.clone();
|
||||
let auto_deny = auto_deny.clone();
|
||||
let reviewer = reviewer.clone();
|
||||
async move {
|
||||
let auto_deny = auto_deny
|
||||
.lock()
|
||||
@@ -138,6 +160,17 @@ impl ElicitationRequestManager {
|
||||
});
|
||||
}
|
||||
|
||||
if let Some(reviewer) = reviewer.as_ref() {
|
||||
let request = ElicitationReviewRequest {
|
||||
server_name: server_name.clone(),
|
||||
request_id: id.clone(),
|
||||
elicitation: elicitation.clone(),
|
||||
};
|
||||
if let Some(response) = reviewer.review(request).await? {
|
||||
return Ok(response);
|
||||
}
|
||||
}
|
||||
|
||||
let request = match elicitation {
|
||||
CreateElicitationRequestParams::FormElicitationParams {
|
||||
meta,
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
pub use connection_manager::McpConnectionManager;
|
||||
pub use elicitation::ElicitationReviewRequest;
|
||||
pub use elicitation::ElicitationReviewer;
|
||||
pub use elicitation::ElicitationReviewerHandle;
|
||||
pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY;
|
||||
pub use runtime::McpRuntimeEnvironment;
|
||||
pub use runtime::SandboxState;
|
||||
|
||||
@@ -264,6 +264,7 @@ pub async fn read_mcp_resource(
|
||||
host_owned_codex_apps_enabled,
|
||||
tool_plugin_provenance(config),
|
||||
auth,
|
||||
/*elicitation_reviewer*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -331,6 +332,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
|
||||
host_owned_codex_apps_enabled,
|
||||
tool_plugin_provenance,
|
||||
auth,
|
||||
/*elicitation_reviewer*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user