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

Honor attachment-owned permissions for MCP servers (#40728)

Browse Source 
## Why

MCP servers attached to executor environments must retain their owner's
permission profile instead of inheriting the thread-wide sandbox authority.

## What changed

- Resolve and capture a permission profile for each enabled MCP server when
  publishing the runtime, and reject calls or elicitations when that authority
  is unavailable.
- Use the captured server profile for tool approval decisions, elicitation, and
  sandbox metadata, including after runtime refreshes.
- Materialize `:workspace_roots` from `PathUri` values so permissions preserve
  the path convention of remote executor environments.
- Give threadless app discovery and resource reads an explicit default
  permission profile.

## Testing

Added coverage for per-server elicitation authority, unresolved attachments,
runtime refreshes, restricted tool calls, and foreign-platform workspace roots.

GitOrigin-RevId: 6b188d4b08b29e9971ff7aa68a7785a0cdf9e394
This commit is contained in:
sayan-oai
2026-08-25 22:50:49 +00:00
committed by copyberry
parent 0b94751cc4
commit 4213b38f3c
25 changed files with 805 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/src/request_processors/apps_processor/installed.rs
+2 -5
View File
@@ -14,7 +14,6 @@ use codex_mcp::effective_mcp_servers;
use codex_mcp::host_owned_codex_apps_enabled;
use codex_mcp::tool_is_model_visible;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::models::PermissionProfile;
#[cfg(test)]
#[path = "installed_tests.rs"]
@@ -57,12 +56,10 @@ impl AppsRequestProcessor {
.apps_enabled_for_auth(auth.as_ref().is_some_and(CodexAuth::uses_codex_backend));
let mcp_manager = self.thread_manager.mcp_manager();
let mut mcp_config = mcp_manager.runtime_config(&config).await;
// Installed-app discovery has no active turn or reviewer.
mcp_config.permission_profile = PermissionProfile::default();
let mcp_config = Arc::new(mcp_config);
let mcp_config = mcp_manager.runtime_config(&config).await;
let mut mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref());
mcp_servers.retain(|name, _| name == CODEX_APPS_MCP_SERVER_NAME);
let mcp_config = Arc::new(mcp_config.for_threadless_operations(&mcp_servers));
let cache_key = connector_runtime_context_key(auth.as_ref());
let previous_snapshot = mcp_manager
.codex_apps_tools_cache()
codex-rs/app-server/tests/suite/v2/mcp_resource.rs
+2 -7
View File
@@ -36,7 +36,6 @@ use codex_protocol::protocol::SessionSource;
use core_test_support::responses;
use pretty_assertions::assert_eq;
use rmcp::handler::server::ServerHandler;
use rmcp::model::BooleanSchema;
use rmcp::model::CallToolRequestParams;
use rmcp::model::CallToolResult;
use rmcp::model::ElicitRequestParams;
@@ -47,7 +46,6 @@ use rmcp::model::ListResourcesResult;
use rmcp::model::ListToolsResult;
use rmcp::model::MetaObject;
use rmcp::model::PaginatedRequestParams;
use rmcp::model::PrimitiveSchemaDefinition;
use rmcp::model::ProtocolVersion;
use rmcp::model::ReadResourceRequestParams;
use rmcp::model::ReadResourceResult;
@@ -569,7 +567,8 @@ async fn mcp_resource_read_returns_contents_and_declines_elicitation_without_thr
r#"
chatgpt_base_url = "{apps_server_url}"
mcp_oauth_credentials_store = "file"
approval_policy = "on-request"
approval_policy = "never"
sandbox_mode = "danger-full-access"
[features]
apps = true
@@ -979,10 +978,6 @@ impl ServerHandler for ResourceAppsMcpServer {
}
if uri == TEST_ELICITATION_RESOURCE_URI {
let requested_schema = ElicitationSchema::builder()
.required_property(
"confirmed",
PrimitiveSchemaDefinition::Boolean(BooleanSchema::new()),
)
.build()
.map_err(|err| rmcp::ErrorData::internal_error(err.to_string(), None))?;
let result = context
codex-rs/codex-mcp/src/binding.rs
+14 -3
View File
@@ -10,6 +10,7 @@ use anyhow::Context;
use anyhow::Result;
use codex_config::AppToolApproval;
use codex_protocol::mcp::CallToolResult;
use codex_protocol::models::PermissionProfile;
use rmcp::model::ListResourceTemplatesResult;
use rmcp::model::ListResourcesResult;
use rmcp::model::PaginatedRequestParams;
@@ -196,9 +197,10 @@ impl PreparedMcpCall {
server_metadata: McpServerMetadata,
plugin_id: Option<String>,
selected_plugin_server: bool,
) -> Self {
) -> Option<Self> {
let server_name = tool_info.server_name.clone();
Self {
config.permission_profile_for_server(&server_name)?;
Some(Self {
_connections: connections,
client,
config,
@@ -209,7 +211,7 @@ impl PreparedMcpCall {
server_metadata,
plugin_id,
selected_plugin_server,
}
})
}
pub fn tool_info(&self) -> &ToolInfo {
@@ -221,6 +223,15 @@ impl PreparedMcpCall {
&self.config
}
/// Returns the owner permissions validated when this immutable call was prepared.
pub fn permission_profile(&self) -> &PermissionProfile {
let Some(permission_profile) = self.config.permission_profile_for_server(&self.server_name)
else {
unreachable!("prepared MCP calls retain their immutable permission authority");
};
permission_profile
}
pub fn server_name(&self) -> &str {
&self.server_name
}
codex-rs/codex-mcp/src/binding_tests.rs
+6 -2
View File
@@ -105,6 +105,9 @@ async fn test_step(
} else {
config.approvals_reviewer = ApprovalsReviewer::AutoReview;
}
config
.server_permission_profiles
.insert(SERVER_NAME.to_string(), config.permission_profile.clone());
let config = Arc::new(config);
let prepared = PreparedMcpCall::new(
Arc::clone(&connections),
@@ -125,7 +128,8 @@ async fn test_step(
},
Some(format!("{label}-plugin")),
label == "old",
);
)
.expect("test call should retain its thread-owned permission profile");
let calls = HashMap::from([((SERVER_NAME.to_string(), TOOL_NAME.to_string()), prepared)]);
TestStep {
@@ -213,7 +217,7 @@ async fn prepared_call_keeps_captured_connection_and_authority_after_refresh() -
assert_eq!(
(
old_call.config().approval_policy.value(),
&old_call.config().permission_profile,
old_call.permission_profile(),
old_call.config().approvals_reviewer,
),
(
codex-rs/codex-mcp/src/connection_manager.rs
+3 -15
View File
@@ -61,8 +61,6 @@ use codex_diagnostics::Gauge;
use codex_diagnostics::GaugeGuard;
use codex_protocol::mcp::CallToolResult;
use codex_protocol::mcp::McpServerInfo;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::McpStartupCompleteEvent;
@@ -226,8 +224,6 @@ impl McpConnectionSet {
} = input;
let store_mode = config.mcp_oauth_credentials_store_mode;
let keyring_backend_kind = config.auth_keyring_backend_kind;
let approval_policy = &config.approval_policy;
let initial_permission_profile = config.permission_profile.clone();
let codex_home = config.codex_home.clone();
let prefix_mcp_tool_names = config.prefix_mcp_tool_names;
let non_prefixed_mcp_tool_servers = config.non_prefixed_mcp_tool_servers.clone();
@@ -255,8 +251,7 @@ impl McpConnectionSet {
let reusable_previous = previous.filter(|previous| {
!previous.servers.is_empty()
&& previous.elicitation_requests.update(
approval_policy.value(),
initial_permission_profile.clone(),
Arc::clone(&config),
elicitation_reviewer.clone(),
elicitation_lifecycle.clone(),
)
@@ -265,8 +260,7 @@ impl McpConnectionSet {
previous.elicitation_requests.clone()
} else {
ElicitationRequestManager::new(
approval_policy.value(),
initial_permission_profile,
Arc::clone(&config),
elicitation_reviewer,
elicitation_lifecycle,
elicitation_router,
@@ -747,13 +741,7 @@ impl McpConnectionSet {
tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()),
prefix_mcp_tool_names,
non_prefixed_mcp_tool_servers: Vec::new(),
elicitation_requests: ElicitationRequestManager::new(
AskForApproval::Never,
PermissionProfile::default(),
/*reviewer*/ None,
/*lifecycle*/ None,
ElicitationRequestRouter::default(),
),
elicitation_requests: ElicitationRequestManager::default(),
}
}
codex-rs/codex-mcp/src/connection_manager/tool_catalog.rs
+2 -2
View File
@@ -334,7 +334,7 @@ impl McpConnectionSet {
) -> Option<PreparedMcpCall> {
let server_name = &tool_info.server_name;
let view = self.servers.get(server_name)?;
Some(PreparedMcpCall::new(
PreparedMcpCall::new(
Arc::clone(self),
client,
config,
@@ -345,7 +345,7 @@ impl McpConnectionSet {
self.plugin_id_for_mcp_server_name(server_name)
.map(str::to_string),
self.is_selected_plugin_mcp_server(server_name),
))
)
}
/// Force-refresh Codex Apps tools and publish one new exact catalog revision.
codex-rs/codex-mcp/src/connection_manager_tests.rs
+64 -31
View File
@@ -6,6 +6,7 @@ use crate::elicitation::ElicitationRequestRouter;
use crate::elicitation::ElicitationReviewRequest;
use crate::elicitation::ElicitationReviewer;
use crate::elicitation::elicitation_is_rejected_by_policy;
use crate::mcp::tests::test_elicitation_config;
use crate::rmcp_client::AsyncManagedClient;
use crate::rmcp_client::CODEX_APPS_RECONNECT_INITIAL_BACKOFF;
use crate::rmcp_client::CodexAppsStartupReconnect;
@@ -43,6 +44,7 @@ use codex_protocol::approvals::ElicitationRequest;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::mcp::McpServerInfo;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::McpStartupFailureReason;
use codex_rmcp_client::ElicitationResponse;
@@ -104,8 +106,11 @@ impl McpConnectionSet {
prefix_mcp_tool_names,
non_prefixed_mcp_tool_servers: Vec::new(),
elicitation_requests: ElicitationRequestManager::new(
approval_policy.value(),
permission_profile.get().clone(),
test_elicitation_config(
"server",
approval_policy.value(),
permission_profile.get().clone(),
),
/*reviewer*/ None,
/*lifecycle*/ None,
ElicitationRequestRouter::default(),
@@ -205,9 +210,15 @@ fn store_current_tools(cache_context: &ConnectorRuntimeContext<ToolInfo>, tools:
}
async fn capture_binding(manager: &Arc<McpConnectionSet>) -> McpBinding {
let mut config = crate::mcp::tests::test_mcp_config(std::env::temp_dir());
config.server_permission_profiles = manager
.servers
.keys()
.map(|name| (name.clone(), PermissionProfile::default()))
.collect();
manager
.capture_binding_with_metadata(
Arc::new(crate::mcp::tests::test_mcp_config(std::env::temp_dir())),
Arc::new(config),
/*plugins_available*/ false,
/*required_servers*/ &[],
)
@@ -787,8 +798,7 @@ 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,
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
/*reviewer*/ None,
/*lifecycle*/ None,
ElicitationRequestRouter::default(),
@@ -822,8 +832,7 @@ 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,
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
/*reviewer*/ None,
/*lifecycle*/ None,
ElicitationRequestRouter::default(),
@@ -914,8 +923,7 @@ async fn assert_elicitation_declined_with_reviewer_calls(
) {
let reviewer = Arc::new(DecliningElicitationReviewer::default());
let manager = ElicitationRequestManager::new(
approval_policy,
PermissionProfile::Disabled,
test_elicitation_config(server_name, approval_policy, PermissionProfile::Disabled),
Some(reviewer.clone()),
/*lifecycle*/ None,
full_access_form_input_enabled_router(),
@@ -955,8 +963,7 @@ async fn assert_requested_user_input_is_declined(
router: ElicitationRequestRouter,
) {
let manager = ElicitationRequestManager::new(
approval_policy,
permission_profile,
test_elicitation_config("server", approval_policy, permission_profile),
/*reviewer*/ None,
/*lifecycle*/ None,
router,
@@ -1043,8 +1050,7 @@ async fn assert_disabled_permissions_surface_requested_user_input(
let router = full_access_form_input_enabled_router();
let reviewer = Arc::new(DecliningElicitationReviewer::default());
let manager = ElicitationRequestManager::new(
AskForApproval::Never,
PermissionProfile::Disabled,
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
Some(reviewer.clone()),
/*lifecycle*/ None,
router.clone(),
@@ -1190,8 +1196,7 @@ async fn on_request_approval_forms_remain_with_the_reviewer() {
#[tokio::test]
async fn disabled_permissions_decline_user_input_without_an_event_channel() {
let manager = ElicitationRequestManager::new(
AskForApproval::Never,
PermissionProfile::Disabled,
test_elicitation_config("server", AskForApproval::Never, PermissionProfile::Disabled),
/*reviewer*/ None,
/*lifecycle*/ None,
full_access_form_input_enabled_router(),
@@ -1222,8 +1227,11 @@ async fn disabled_permissions_decline_user_input_without_an_event_channel() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn concurrent_authority_updates_never_auto_approve_mixed_policy() {
let manager = ElicitationRequestManager::new(
AskForApproval::Never,
PermissionProfile::default(),
test_elicitation_config(
"server",
AskForApproval::Never,
PermissionProfile::default(),
),
/*reviewer*/ None,
/*lifecycle*/ None,
ElicitationRequestRouter::default(),
@@ -1232,14 +1240,20 @@ async fn concurrent_authority_updates_never_auto_approve_mixed_policy() {
let updater = tokio::spawn(async move {
for _ in 0..1_000 {
assert!(updating_manager.update(
AskForApproval::OnRequest,
PermissionProfile::Disabled,
test_elicitation_config(
"server",
AskForApproval::OnRequest,
PermissionProfile::Disabled
),
/*reviewer*/ None,
/*lifecycle*/ None,
));
assert!(updating_manager.update(
AskForApproval::Never,
PermissionProfile::default(),
test_elicitation_config(
"server",
AskForApproval::Never,
PermissionProfile::default()
),
/*reviewer*/ None,
/*lifecycle*/ None,
));
@@ -1292,15 +1306,21 @@ async fn shared_elicitation_router_targets_the_exact_pending_request() {
}
});
let manager_a = ElicitationRequestManager::new(
AskForApproval::OnRequest,
PermissionProfile::default(),
test_elicitation_config(
"server",
AskForApproval::OnRequest,
PermissionProfile::default(),
),
/*reviewer*/ None,
Some(lifecycle.clone()),
router.clone(),
);
let manager_b = ElicitationRequestManager::new(
AskForApproval::OnRequest,
PermissionProfile::default(),
test_elicitation_config(
"server",
AskForApproval::OnRequest,
PermissionProfile::default(),
),
/*reviewer*/ None,
Some(lifecycle),
router.clone(),
@@ -1392,8 +1412,11 @@ async fn shared_elicitation_router_targets_the_exact_pending_request() {
async fn cancelled_elicitation_is_removed_without_affecting_other_pending_requests() {
let router = ElicitationRequestRouter::default();
let manager = ElicitationRequestManager::new(
AskForApproval::OnRequest,
PermissionProfile::default(),
test_elicitation_config(
"server",
AskForApproval::OnRequest,
PermissionProfile::default(),
),
/*reviewer*/ None,
/*lifecycle*/ None,
router.clone(),
@@ -4849,8 +4872,14 @@ async fn reconciliation_updates_elicitation_policy_without_restarting_ready_serv
.authority
.lock()
.expect("elicitation authority lock");
authority.approval_policy = AskForApproval::Never;
authority.permission_profile = PermissionProfile::Disabled;
let config = Arc::make_mut(
&mut authority
.as_mut()
.expect("test manager should have permission authority")
.config,
);
config.approval_policy = Constrained::allow_any(AskForApproval::Never);
config.permission_profile = PermissionProfile::Disabled;
}
let reconciled = reconcile_reusable_server(&previous, config, runtime_context).await;
@@ -4861,8 +4890,12 @@ async fn reconciliation_updates_elicitation_policy_without_restarting_ready_serv
.authority
.lock()
.expect("elicitation authority lock");
assert_eq!(authority.approval_policy, AskForApproval::OnRequest);
assert_eq!(authority.permission_profile, PermissionProfile::default());
let config = &authority
.as_ref()
.expect("reconciled manager should have permission authority")
.config;
assert_eq!(config.approval_policy.value(), AskForApproval::OnRequest);
assert_eq!(config.permission_profile, PermissionProfile::default());
}
#[tokio::test]
codex-rs/codex-mcp/src/elicitation.rs
+25 -21
View File
@@ -12,6 +12,7 @@ use std::sync::atomic::AtomicBool;
use std::sync::atomic::AtomicU64;
use std::sync::atomic::Ordering;
use crate::McpConfig;
use crate::mcp::McpPermissionPromptAutoApproveContext;
use crate::mcp::mcp_permission_prompt_is_auto_approved;
use anyhow::Context;
@@ -25,7 +26,6 @@ use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY;
use codex_protocol::mcp_approval_meta::APPROVAL_KIND_TOOL_SUGGESTION;
use codex_protocol::mcp_approval_meta::APPROVALS_REVIEWER_KEY;
use codex_protocol::mcp_approval_meta::STRICT_AUTO_REVIEW_KEY;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -153,53 +153,48 @@ impl ElicitationRequestRouter {
#[derive(Clone)]
pub(crate) struct ElicitationAuthority {
pub(crate) approval_policy: AskForApproval,
pub(crate) permission_profile: PermissionProfile,
pub(crate) config: Arc<McpConfig>,
reviewer: Option<ElicitationReviewerHandle>,
lifecycle: Option<ElicitationLifecycle>,
}
#[derive(Clone)]
#[derive(Clone, Default)]
pub(crate) struct ElicitationRequestManager {
router: ElicitationRequestRouter,
pub(crate) authority: Arc<StdMutex<ElicitationAuthority>>,
pub(crate) authority: Arc<StdMutex<Option<ElicitationAuthority>>>,
}
impl ElicitationRequestManager {
pub(crate) fn new(
approval_policy: AskForApproval,
permission_profile: PermissionProfile,
config: Arc<McpConfig>,
reviewer: Option<ElicitationReviewerHandle>,
lifecycle: Option<ElicitationLifecycle>,
router: ElicitationRequestRouter,
) -> Self {
Self {
router,
authority: Arc::new(StdMutex::new(ElicitationAuthority {
approval_policy,
permission_profile,
authority: Arc::new(StdMutex::new(Some(ElicitationAuthority {
config,
reviewer,
lifecycle,
})),
}))),
}
}
pub(crate) fn update(
&self,
approval_policy: AskForApproval,
permission_profile: PermissionProfile,
config: Arc<McpConfig>,
reviewer: Option<ElicitationReviewerHandle>,
lifecycle: Option<ElicitationLifecycle>,
) -> bool {
let Ok(mut authority) = self.authority.lock() else {
return false;
};
*authority = ElicitationAuthority {
approval_policy,
permission_profile,
*authority = Some(ElicitationAuthority {
config,
reviewer,
lifecycle,
};
});
true
}
@@ -237,7 +232,8 @@ impl ElicitationRequestManager {
});
}
let Ok(authority) = authority.lock().map(|authority| authority.clone()) else {
let Ok(Some(authority)) = authority.lock().map(|authority| authority.clone())
else {
return Ok(ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
@@ -245,11 +241,19 @@ impl ElicitationRequestManager {
});
};
let ElicitationAuthority {
approval_policy,
permission_profile,
config,
reviewer,
lifecycle,
} = authority;
let approval_policy = config.approval_policy.value();
let Some(permission_profile) = config.permission_profile_for_server(&server_name)
else {
return Ok(ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
meta: None,
});
};
match elicitation
.meta()
@@ -303,7 +307,7 @@ impl ElicitationRequestManager {
let permission_prompt_is_auto_approved = mcp_permission_prompt_is_auto_approved(
approval_policy,
&permission_profile,
permission_profile,
McpPermissionPromptAutoApproveContext::default(),
);
if permission_prompt_is_auto_approved && can_auto_accept_elicitation(&elicitation) {
codex-rs/codex-mcp/src/elicitation_tests.rs
+119 -4
View File
@@ -1,5 +1,7 @@
use super::*;
use crate::mcp::tests::test_elicitation_config;
use async_channel::Receiver;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::GranularApprovalConfig;
use pretty_assertions::assert_eq;
use rmcp::model::ElicitRequestParams;
@@ -75,9 +77,16 @@ fn elicitation_fixture(
LifecycleRegistration(active_elicitations.clone())
})
});
let manager = ElicitationRequestManager::new(
let mut config = test_elicitation_config(
"independent-mcp",
approval_policy,
permission_profile,
permission_profile.clone(),
);
Arc::make_mut(&mut config)
.server_permission_profiles
.insert("another-independent-mcp".to_string(), permission_profile);
let manager = ElicitationRequestManager::new(
config,
reviewer.map(|reviewer| reviewer as Arc<dyn ElicitationReviewer>),
lifecycle,
ElicitationRequestRouter::default(),
@@ -132,8 +141,11 @@ fn closed_event_channel_immediately_cleans_up_pending_elicitation() {
/*reviewer*/ None,
);
assert!(manager.update(
AskForApproval::OnRequest,
PermissionProfile::Disabled,
test_elicitation_config(
"independent-mcp",
AskForApproval::OnRequest,
PermissionProfile::Disabled
),
/*reviewer*/ None,
Some(lifecycle),
));
@@ -283,3 +295,106 @@ async fn strict_auto_review_fails_closed_without_a_canonical_decision() {
}
assert_declined(json!(true), /*response*/ None).await;
}
#[tokio::test]
async fn reused_elicitation_senders_follow_each_servers_latest_permission_authority() {
let mut config = crate::mcp::tests::test_mcp_config(std::env::temp_dir());
config.approval_policy = codex_config::Constrained::allow_any(AskForApproval::Never);
config.permission_profile = PermissionProfile::Disabled;
config.apps_enabled = true;
let auth = codex_login::CodexAuth::create_dummy_chatgpt_auth_for_testing();
let hosted_server = crate::codex_apps_mcp_server_config(
"https://example.com",
/*apps_mcp_product_sku*/ None,
/*originator*/ None,
);
let mut attached_server = hosted_server.clone();
attached_server.environment_id = "attached".to_string();
let mut catalog = crate::ResolvedMcpCatalog::builder();
catalog.register(crate::McpServerRegistration::from_config(
"attached".to_string(),
attached_server,
));
catalog.register(crate::McpServerRegistration::from_hosted_apps(
"host",
/*contribution_order*/ 0,
hosted_server,
));
config.mcp_server_catalog = catalog.build();
let servers = crate::effective_mcp_servers(&config, Some(&auth));
config.set_server_permission_profiles(
&servers,
[("attached".to_string(), PermissionProfile::read_only())],
);
let manager = ElicitationRequestManager::new(
Arc::new(config.clone()),
/*reviewer*/ None,
/*lifecycle*/ None,
ElicitationRequestRouter::default(),
);
let attached = manager.make_sender("attached".to_string(), /*tx_event*/ None);
let hosted = manager.make_sender(
crate::CODEX_APPS_MCP_SERVER_NAME.to_string(),
/*tx_event*/ None,
);
assert_eq!(
send_elicitation(&attached, /*marker*/ None).await.action,
ElicitationAction::Decline
);
assert_eq!(
send_elicitation(&hosted, /*marker*/ None).await.action,
ElicitationAction::Accept
);
config.set_server_permission_profiles(
&servers,
[("attached".to_string(), PermissionProfile::Disabled)],
);
assert!(manager.update(
Arc::new(config.clone()),
/*reviewer*/ None,
/*lifecycle*/ None,
));
assert_eq!(
send_elicitation(&attached, /*marker*/ None).await.action,
ElicitationAction::Accept
);
let mut configured_servers = config.mcp_server_catalog.configured_servers();
configured_servers
.get_mut("attached")
.expect("attached server should be registered")
.enabled = false;
config.mcp_server_catalog = config
.mcp_server_catalog
.with_materialized_servers(configured_servers);
let servers = crate::effective_mcp_servers(&config, Some(&auth));
config.set_server_permission_profiles(
&servers,
[("attached".to_string(), PermissionProfile::Disabled)],
);
assert!(manager.update(
Arc::new(config.clone()),
/*reviewer*/ None,
/*lifecycle*/ None,
));
assert_eq!(
send_elicitation(&attached, /*marker*/ None).await.action,
ElicitationAction::Decline
);
let servers = crate::effective_mcp_servers(&config, /*auth*/ None);
config.set_server_permission_profiles(&servers, std::iter::empty());
assert!(manager.update(
Arc::new(config.clone()),
/*reviewer*/ None,
/*lifecycle*/ None,
));
assert_eq!(
send_elicitation(&hosted, /*marker*/ None).await.action,
ElicitationAction::Decline
);
}
codex-rs/codex-mcp/src/mcp/mod.rs
+57 -4
View File
@@ -49,6 +49,7 @@ use serde_json::Value;
use tokio_util::sync::CancellationToken;
use crate::McpProtocolMode;
use crate::McpServerSource;
use crate::ResolvedMcpCatalog;
use crate::connection_manager::McpConnectionSet;
use crate::runtime::McpPublicationGate;
@@ -145,6 +146,8 @@ pub struct McpConfig {
pub approvals_reviewer: ApprovalsReviewer,
/// Working directories for the exact environment handles used by this runtime.
pub environment_cwds: HashMap<String, PathUri>,
/// Explicit server permissions; unresolved or unavailable servers have no entry.
pub server_permission_profiles: HashMap<String, PermissionProfile>,
/// Optional path to `codex-linux-sandbox` for sandboxed MCP tool execution.
pub codex_linux_sandbox_exe: Option<PathBuf>,
/// Whether to use legacy Landlock behavior in the MCP sandbox state.
@@ -172,6 +175,58 @@ pub struct McpConfig {
pub connector_snapshot: ConnectorSnapshot,
}
impl McpConfig {
/// Resolves enabled runtime servers against the exact attachment permissions being published.
pub fn set_server_permission_profiles(
&mut self,
servers: &HashMap<String, EffectiveMcpServer>,
environment_profiles: impl IntoIterator<Item = (String, PermissionProfile)>,
) {
let environment_profiles = environment_profiles.into_iter().collect::<HashMap<_, _>>();
self.server_permission_profiles = servers
.iter()
.filter(|(_, server)| server.enabled())
.filter_map(|(server_name, _)| {
let server = self.mcp_server_catalog.server(server_name)?;
let permission_profile = if server
.source()
.is_host_owned_apps(server_name, server.config())
{
&self.permission_profile
} else if let Some(permission_profile) =
environment_profiles.get(&server.config().environment_id)
{
permission_profile
} else if server.config().is_local_environment()
|| matches!(server.source(), McpServerSource::SelectedPlugin(_))
{
&self.permission_profile
} else {
return None;
};
Some((server_name.clone(), permission_profile.clone()))
})
.collect();
}
/// Returns this server's published permission authority.
pub fn permission_profile_for_server(&self, server_name: &str) -> Option<&PermissionProfile> {
self.server_permission_profiles.get(server_name)
}
/// Standalone discovery and resource reads must not inherit thread execution authority.
pub fn for_threadless_operations(&self, servers: &HashMap<String, EffectiveMcpServer>) -> Self {
let mut config = self.clone();
config.permission_profile = PermissionProfile::default();
config.server_permission_profiles = servers
.iter()
.filter(|(_, server)| server.enabled())
.map(|(name, _)| (name.clone(), PermissionProfile::default()))
.collect();
config
}
}
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct ToolPluginProvenance {
plugin_display_names_by_connector_id: HashMap<String, Vec<String>>,
@@ -357,8 +412,7 @@ pub async fn read_mcp_resource(
let mut mcp_servers = effective_mcp_servers(config, auth);
mcp_servers.retain(|name, _| name == server);
let cancel_token = CancellationToken::new();
let mut runtime_config = config.clone();
runtime_config.permission_profile = PermissionProfile::default();
let runtime_config = config.for_threadless_operations(&mcp_servers);
let manager = McpConnectionSet::new(
/*previous*/ None,
McpPublicationGate::already_published(),
@@ -433,8 +487,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
let server_names = mcp_servers.keys().cloned().collect();
let cancel_token = CancellationToken::new();
let mut runtime_config = config.clone();
runtime_config.permission_profile = PermissionProfile::default();
let runtime_config = config.for_threadless_operations(&mcp_servers);
let mcp_connection_manager = McpConnectionSet::new(
/*previous*/ None,
McpPublicationGate::already_published(),
codex-rs/codex-mcp/src/mcp/mod_tests.rs
+57
View File
@@ -16,6 +16,7 @@ use pretty_assertions::assert_eq;
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::PathBuf;
use std::sync::Arc;
pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
McpConfig {
@@ -32,6 +33,7 @@ pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
config_layer_stack: codex_config::ConfigLayerStack::default(),
approvals_reviewer: codex_config::types::ApprovalsReviewer::default(),
environment_cwds: HashMap::new(),
server_permission_profiles: HashMap::new(),
codex_linux_sandbox_exe: None,
use_legacy_landlock: false,
apps_enabled: false,
@@ -44,6 +46,20 @@ pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
}
}
pub(crate) fn test_elicitation_config(
server_name: &str,
approval_policy: AskForApproval,
permission_profile: PermissionProfile,
) -> Arc<McpConfig> {
let mut config = test_mcp_config(PathBuf::new());
config.approval_policy = Constrained::allow_any(approval_policy);
config.permission_profile = permission_profile.clone();
config
.server_permission_profiles
.insert(server_name.to_string(), permission_profile);
Arc::new(config)
}
#[test]
fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() {
assert_eq!(
@@ -52,6 +68,47 @@ fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() {
);
}
#[test]
fn mcp_server_permissions_handle_unattached_and_threadless_servers() {
let mut config = test_mcp_config(PathBuf::new());
config.permission_profile = PermissionProfile::Disabled;
let mut missing_server = codex_apps_mcp_server_config(
"https://example.com",
/*apps_mcp_product_sku*/ None,
/*originator*/ None,
);
missing_server.environment_id = "missing".to_string();
let mut selected_server = missing_server.clone();
selected_server.environment_id = "unattached".to_string();
let mut catalog = ResolvedMcpCatalog::builder();
catalog.register(McpServerRegistration::from_config(
"missing".to_string(),
missing_server,
));
catalog.register(McpServerRegistration::from_selected_plugin(
"selected".to_string(),
McpPluginAttribution::new("selected@test".to_string(), "Selected".to_string()),
/*selection_order*/ 0,
selected_server,
));
config.mcp_server_catalog = catalog.build();
let servers = effective_mcp_servers(&config, /*auth*/ None);
assert_eq!(config.permission_profile_for_server("selected"), None);
config.set_server_permission_profiles(&servers, std::iter::empty());
assert_eq!(
config.permission_profile_for_server("selected"),
Some(&PermissionProfile::Disabled)
);
assert_eq!(config.permission_profile_for_server("missing"), None);
let config = config.for_threadless_operations(&servers);
assert_eq!(
config.permission_profile_for_server("selected"),
Some(&PermissionProfile::default())
);
}
#[test]
fn mcp_prompt_auto_approval_honors_unrestricted_managed_profiles() {
assert!(mcp_permission_prompt_is_auto_approved(
codex-rs/codex-mcp/src/runtime.rs
+21
View File
@@ -104,6 +104,7 @@ struct PublishedMcpRuntime {
auth_token: Option<String>,
plugins_available: bool,
ready_selected_capability_roots: Vec<SelectedCapabilityRoot>,
selected_environments: HashMap<String, Arc<Environment>>,
cached_binding: Mutex<Option<CachedMcpBinding>>,
}
@@ -174,6 +175,7 @@ impl McpRuntime {
auth_token: None,
plugins_available: false,
ready_selected_capability_roots: Vec::new(),
selected_environments: HashMap::new(),
cached_binding: Mutex::new(None),
}),
hosted_event_server_removals: watch::channel(()).0,
@@ -270,6 +272,7 @@ impl McpRuntime {
let auth_token = auth.as_ref().and_then(|auth| auth.get_token().ok());
let plugins_available = input.plugins_available;
let ready_selected_capability_roots = input.ready_selected_capability_roots.clone();
let selected_environments = input.runtime_context.selected_environments.clone();
let connections = Arc::new(
McpConnectionSet::new(
previous,
@@ -295,6 +298,7 @@ impl McpRuntime {
auth_token,
plugins_available,
ready_selected_capability_roots,
selected_environments,
cached_binding: Mutex::new(None),
}));
let _ = publish.send(true);
@@ -432,6 +436,22 @@ impl McpRuntime {
self.current.load().ready_selected_capability_roots.clone()
}
/// Whether this publication uses the currently ready environment handles.
pub fn current_environments_match(
&self,
environments: &HashMap<String, Arc<Environment>>,
) -> bool {
let current = self.current.load();
current.config.is_some()
&& current.selected_environments.len() == environments.len()
&& environments.iter().all(|(id, environment)| {
current
.selected_environments
.get(id)
.is_some_and(|published| Arc::ptr_eq(published, environment))
})
}
pub fn elicitations_auto_deny(&self) -> bool {
self.elicitation_router.auto_deny()
}
@@ -789,6 +809,7 @@ mod tests {
auth_token: None,
plugins_available: false,
ready_selected_capability_roots: Vec::new(),
selected_environments: HashMap::new(),
cached_binding: Mutex::new(None),
});
let first = McpRuntime::binding_from_published_runtime(
codex-rs/core/src/config/mod.rs
+1
View File
@@ -1725,6 +1725,7 @@ impl Config {
config_layer_stack: self.config_layer_stack.clone(),
approvals_reviewer: self.approvals_reviewer,
environment_cwds: HashMap::new(),
server_permission_profiles: HashMap::new(),
codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(),
use_legacy_landlock: self.features.use_legacy_landlock(),
apps_enabled: self.features.enabled(Feature::Apps),
codex-rs/core/src/connectors.rs
+2 -5
View File
@@ -41,7 +41,6 @@ use codex_mcp::ToolPluginProvenance;
use codex_mcp::effective_mcp_servers;
use codex_mcp::tool_plugin_provenance;
use codex_protocol::mcp::ClientMcpExtensions;
use codex_protocol::models::PermissionProfile;
const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30);
@@ -224,10 +223,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
});
}
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
let mut mcp_config = mcp_manager.runtime_config(config).await;
// Discovery has no active turn or reviewer and must never inherit execution authority.
mcp_config.permission_profile = PermissionProfile::default();
let mcp_config = Arc::new(mcp_config);
let mcp_config = mcp_manager.runtime_config(config).await;
let tool_plugin_provenance = tool_plugin_provenance(&mcp_config);
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
{
@@ -240,6 +236,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
let mut mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref());
mcp_servers.retain(|name, _| name == CODEX_APPS_MCP_SERVER_NAME);
let mcp_config = Arc::new(mcp_config.for_threadless_operations(&mcp_servers));
if mcp_servers.is_empty() {
return Ok(AccessibleConnectorsStatus {
connectors: Vec::new(),
codex-rs/core/src/mcp_tool_call.rs
+5 -3
View File
@@ -59,6 +59,7 @@ use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_TOOL_APPROVAL
use codex_protocol::mcp_approval_meta::TOOL_PARAMS_DISPLAY_KEY as MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY;
use codex_protocol::mcp_approval_meta::TOOL_PARAMS_KEY as MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY;
use codex_protocol::mcp_approval_meta::TOOL_TITLE_KEY as MCP_TOOL_APPROVAL_TOOL_TITLE_KEY;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::InputModality;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::McpInvocation;
@@ -243,6 +244,7 @@ pub(crate) async fn handle_mcp_tool_call(
&hook_tool_name,
&metadata,
prepared_call.config(),
prepared_call.permission_profile(),
approval_policy,
)
.await
@@ -775,9 +777,8 @@ async fn augment_mcp_tool_request_meta_with_sandbox_state(
};
// TODO(anp): Build this metadata from the server's captured
// TurnEnvironment::sandbox_context instead of the runtime-wide Landlock value.
let permission_profile = prepared_call.config().permission_profile.clone();
let sandbox_state = serde_json::to_value(SandboxState {
permission_profile,
permission_profile: prepared_call.permission_profile().clone(),
codex_linux_sandbox_exe: prepared_call.config().codex_linux_sandbox_exe.clone(),
sandbox_cwd,
use_legacy_landlock: prepared_call.config().use_legacy_landlock,
@@ -1301,6 +1302,7 @@ async fn maybe_request_mcp_tool_approval(
hook_tool_name: &HookToolName,
metadata: &McpToolApprovalMetadata,
config: &codex_mcp::McpConfig,
permission_profile: &PermissionProfile,
policy: McpToolApprovalPolicy,
) -> Option<ReviewDecision> {
let turn_context = &step_context.turn;
@@ -1324,7 +1326,7 @@ async fn maybe_request_mcp_tool_approval(
if !strict_auto_review
&& mcp_permission_prompt_is_auto_approved(
config.approval_policy.value(),
&config.permission_profile,
permission_profile,
McpPermissionPromptAutoApproveContext {
tool_approval_mode: Some(policy.mode),
},
codex-rs/core/src/mcp_tool_call_tests.rs
+10
View File
@@ -2335,6 +2335,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
&HookToolName::new("mcp__test__tool"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Approve),
)
.await;
@@ -2415,6 +2416,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
&HookToolName::new("mcp__test__tool"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Auto),
)
.await;
@@ -2477,6 +2479,7 @@ async fn permission_request_hook_allows_mcp_tool_call() {
&HookToolName::new("mcp__memory__create_entities"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Auto),
)
.await;
@@ -2547,6 +2550,7 @@ async fn permission_request_hook_uses_hook_tool_name_without_metadata() {
&HookToolName::new("mcp__memory__create_entities"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Auto),
)
.await;
@@ -2632,6 +2636,7 @@ async fn permission_request_hook_runs_after_remembered_mcp_approval() {
&HookToolName::new("mcp__memory__create_entities"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Auto),
)
.await;
@@ -2731,6 +2736,7 @@ async fn strict_auto_review_forces_guardian_for_mcp_policy_skip() {
&HookToolName::new("mcp__test__tool"),
&metadata,
&captured_mcp_config,
&captured_mcp_config.permission_profile,
McpToolApprovalPolicy::for_server(AppToolApproval::Approve),
)
.await;
@@ -2808,6 +2814,7 @@ async fn assert_mcp_user_approval_persistence(
&HookToolName::new("mcp__memory__create_entities"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Auto),
)
.await
@@ -2893,6 +2900,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
&HookToolName::new("mcp__test__tool"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Prompt),
)
.await
@@ -2958,6 +2966,7 @@ async fn full_access_mode_skips_mcp_tool_approval_for_all_approval_modes() {
&HookToolName::new("mcp__test__tool"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(approval_mode),
)
.await;
@@ -3051,6 +3060,7 @@ async fn approve_mode_skips_guardian_in_every_permission_mode() {
&HookToolName::new("mcp__test__tool"),
&metadata,
&approval_config(&turn_context),
turn_context.config.permissions.permission_profile(),
McpToolApprovalPolicy::for_server(AppToolApproval::Approve),
)
.await;
codex-rs/core/src/session/mcp.rs
+24 -4
View File
@@ -178,6 +178,24 @@ impl Session {
return;
};
loop {
let environments = self.services.turn_environments.snapshot().await;
let ready_environments = environments
.turn_environments()
.map(|environment| {
(
environment.selection.environment_id.clone(),
Arc::clone(&environment.environment),
)
})
.collect();
// Attachment resolution can finish after the owner's configuration callback.
if !self
.services
.mcp_runtime
.current_environments_match(&ready_environments)
{
self.mark_mcp_runtime_dirty();
}
let auth = self.services.auth_manager.auth_cached();
if !self
.services
@@ -232,9 +250,6 @@ impl Session {
)
.await;
refresh_invalidation.published = true;
if !self.mcp_refresh.is_pending() {
return;
}
}
}
@@ -813,9 +828,14 @@ async fn review_guardian_mcp_elicitation(
let approval_policy = mcp_config.approval_policy.value();
match approval_policy {
AskForApproval::Never => {
let Some(permission_profile) =
mcp_config.permission_profile_for_server(&request.server_name)
else {
return Ok(Some(mcp_elicitation_decline_without_message()));
};
if codex_mcp::mcp_permission_prompt_is_auto_approved(
approval_policy,
&mcp_config.permission_profile,
permission_profile,
codex_mcp::McpPermissionPromptAutoApproveContext::default(),
) && matches!(
&request.elicitation,
codex-rs/core/src/session/mcp_refresh.rs
+1
View File
@@ -22,6 +22,7 @@ impl McpRefresh {
self.pending.store(true, Ordering::Release);
}
#[cfg(test)]
pub(super) fn is_pending(&self) -> bool {
self.pending.load(Ordering::Acquire)
}
codex-rs/core/src/session/mcp_runtime.rs
+10 -1
View File
@@ -333,8 +333,17 @@ impl Session {
.environment_cwds
.entry(codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string())
.or_insert_with(|| PathUri::from_abs_path(&desired.config.cwd));
let mcp_servers = effective_mcp_servers(&config, auth.as_ref());
config.set_server_permission_profiles(
&mcp_servers,
desired.environments.turn_environments().map(|environment| {
(
environment.selection.environment_id.clone(),
environment.permission_profile_with_workspace_roots(),
)
}),
);
let mcp_config = Arc::new(config);
let mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref());
let runtime_context = McpRuntimeContext::new(
self.services.turn_environments.environment_manager(),
desired.local_process_cwd.clone(),
codex-rs/core/src/session/tests.rs
+62 -2
View File
@@ -169,6 +169,7 @@ use core_test_support::PathExt;
use core_test_support::context_snapshot;
use core_test_support::context_snapshot::ContextSnapshotOptions;
use core_test_support::context_snapshot::ContextSnapshotRenderMode;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
@@ -8777,7 +8778,33 @@ async fn refreshed_mcp_binding_captures_current_approval_authority() {
#[tokio::test]
async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() {
let (session, old_turn, rx) = make_session_and_context_with_rx().await;
let guardian_server = start_mock_server().await;
mount_sse_once(
&guardian_server,
sse(vec![
ev_response_created("guardian-review"),
ev_assistant_message("guardian-review", r#"{"outcome":"allow"}"#),
ev_completed("guardian-review"),
]),
)
.await;
let (session, old_turn, rx) = make_session_and_context_with_auth_and_config_and_rx(
CodexAuth::from_api_key("Test API Key"),
Vec::new(),
|config| {
config.model_provider.base_url = Some(format!("{}/v1", guardian_server.uri()));
config
.mcp_servers
.set(
serde_json::from_value(json!({
"browser-use": { "command": "missing-test-mcp-server" }
}))
.expect("test MCP server configuration should deserialize"),
)
.expect("test MCP server should be configurable");
},
)
.await;
assert_eq!(old_turn.config.approvals_reviewer, ApprovalsReviewer::User);
session
.spawn_task(
@@ -8870,7 +8897,7 @@ async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() {
assert_eq!(
session
.mcp_elicitation_reviewer()
.review(request)
.review(request.clone())
.await
.expect("elicitation review should succeed"),
Some(ElicitationResponse {
@@ -8880,6 +8907,39 @@ async fn mcp_elicitation_reviewer_uses_latest_runtime_authority() {
})
);
let selection = session
.services
.turn_environments
.selections()
.into_iter()
.next()
.expect("session should select its executor environment");
let mut owner_config = old_turn
.environments
.primary()
.expect("ready environment")
.config()
.clone();
owner_config.permission_profile =
PermissionProfileSnapshot::legacy(PermissionProfile::read_only());
session
.environment_ready(&selection, owner_config)
.await
.expect("attachment owner should install its restricted permissions");
session.refresh_mcp_if_dirty().await;
assert_eq!(
session
.mcp_elicitation_reviewer()
.review(request)
.await
.expect("elicitation review should succeed"),
Some(ElicitationResponse {
action: ElicitationAction::Decline,
content: None,
meta: Some(json!({ "approvals_reviewer": "auto_review" })),
})
);
session.abort_all_tasks(TurnAbortReason::Interrupted).await;
}
codex-rs/core/src/session/turn_context.rs
+1 -6
View File
@@ -143,14 +143,9 @@ impl TurnEnvironment {
}
pub(crate) fn permission_profile_with_workspace_roots(&self) -> PermissionProfile {
let workspace_roots = self
.workspace_roots()
.iter()
.filter_map(|workspace_root| workspace_root.to_abs_path().ok())
.collect::<Vec<_>>();
self.permission_profile()
.clone()
.materialize_project_roots_with_workspace_roots(&workspace_roots)
.materialize_project_roots_with_path_uris(self.workspace_roots())
}
pub(crate) fn selection(&self) -> TurnEnvironmentSelection {
codex-rs/core/tests/suite/remote_env.rs
+72 -2
View File
@@ -871,6 +871,8 @@ async fn settings_update_does_not_retarget_active_turn_environment() -> Result<(
async fn deferred_executor_promotes_primary_environment_when_startup_completes() -> Result<()> {
let listener = TcpListener::bind("127.0.0.1:0").await?;
let server = start_mock_server().await;
let apps = core_test_support::apps_test_server::AppsTestServer::mount(&server).await?;
let mcp_url = format!("{}/api/codex/ps/mcp", apps.chatgpt_base_url);
let response_mock = mount_sse_sequence(
&server,
vec![
@@ -912,7 +914,20 @@ async fn deferred_executor_promotes_primary_environment_when_startup_completes()
.await;
let mut builder = test_codex()
.with_exec_server_url(format!("ws://{}", listener.local_addr()?))
.with_config(|config| {
.with_model_info_override("gpt-5.4", |model| model.supports_search_tool = false)
.with_config(move |config| {
config
.mcp_servers
.set(HashMap::from([(
"deferred".to_string(),
serde_json::from_value(json!({
"url": mcp_url,
"environment_id": REMOTE_ENVIRONMENT_ID,
"startup_timeout_sec": 60,
}))
.expect("MCP server config"),
)]))
.expect("set MCP server");
config.project_doc_max_bytes = 0;
assert!(config.features.enable(Feature::DeferredExecutor).is_ok());
assert!(
@@ -966,8 +981,56 @@ async fn deferred_executor_promotes_primary_environment_when_startup_completes()
assert!(initial_context.contains("<environment id=\"local\" primary=\"true\">"));
assert!(initial_context.contains("<environment id=\"remote\" primary=\"false\">"));
assert!(initial_context.contains("<status>starting</status>"));
assert!(
requests[1]
.tool_by_name("mcp__deferred", "calendar_list_events")
.is_none()
);
serve_environment_info(listener).await;
let mut websocket = accept_initialized_exec_server(listener).await;
// Forward MCP HTTP through the fake executor while keeping startup under test control.
let http_client =
codex_exec_server::Environment::create_for_tests(/*exec_server_url*/ None)?
.get_http_client();
let executor = tokio::spawn(async move {
loop {
let request = read_exec_server_json(&mut websocket).await;
let Some(id) = request.get("id") else {
continue;
};
let mut messages = Vec::new();
if request["method"] == "http/request" {
let params = serde_json::from_value(request["params"].clone()).unwrap();
let mut response =
serde_json::to_value(http_client.http_request(params).await.unwrap()).unwrap();
if request["params"]["streamResponse"] == true {
let body = response["bodyBase64"].take();
response["bodyBase64"] = json!("");
messages.push(json!({
"method": "http/request/bodyDelta",
"params": { "requestId": request["params"]["requestId"], "seq": 1,
"deltaBase64": body, "done": true }
}));
}
messages.insert(0, json!({ "id": id, "result": response }));
} else if request["method"] == "environment/info" {
messages.push(json!({ "id": id,
"result": { "shell": { "name": "zsh", "path": "/bin/zsh" } }
}));
} else {
messages.push(
json!({ "id": id, "error": { "code": -32601, "message": "unsupported" } }),
);
}
for message in messages {
websocket
.send(Message::Text(message.to_string().into()))
.await
.unwrap();
}
}
});
core_test_support::wait_for_mcp_server(&test.codex, "deferred").await?;
test.codex
.submit(Op::UserInputAnswer {
id: request.turn_id,
@@ -987,6 +1050,11 @@ async fn deferred_executor_promotes_primary_environment_when_startup_completes()
.await;
let requests = response_mock.requests();
assert!(
requests[2]
.tool_by_name("mcp__deferred", "calendar_list_events")
.is_some()
);
let updated_context = requests[2]
.message_input_texts("user")
.into_iter()
@@ -1017,6 +1085,8 @@ async fn deferred_executor_promotes_primary_environment_when_startup_completes()
Some(&Value::Null)
);
executor.abort();
let _ = executor.await;
Ok(())
}
codex-rs/core/tests/suite/rmcp_client.rs
+147 -16
View File
@@ -61,6 +61,12 @@ use codex_protocol::openai_models::ModelVisibility;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::openai_models::ReasoningEffortPreset;
use codex_protocol::openai_models::TruncationPolicyConfig;
use codex_protocol::permissions::FileSystemAccessMode;
use codex_protocol::permissions::FileSystemPath;
use codex_protocol::permissions::FileSystemSandboxEntry;
use codex_protocol::permissions::FileSystemSandboxPolicy;
use codex_protocol::permissions::FileSystemSpecialPath;
use codex_protocol::permissions::NetworkSandboxPolicy;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EnvironmentConfigState;
use codex_protocol::protocol::EventMsg;
@@ -1541,14 +1547,16 @@ async fn local_stdio_server_uses_runtime_fallback_cwd_when_config_omits_cwd() ->
Ok(())
}
#[test_case(false, false; "both disabled")]
#[test_case(true, false; "auto review required")]
#[test_case(false, true; "disabled")]
#[test_case(true, true; "both enabled")]
#[test_case(false, false, false; "both disabled")]
#[test_case(true, false, false; "auto review required")]
#[test_case(false, true, false; "disabled")]
#[test_case(true, true, false; "both enabled")]
#[test_case(false, false, true; "attachment-owned permissions preserve foreign workspace roots")]
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
node_repl_auto_review_required: bool,
node_repl_disabled: bool,
attachment_owned_permissions: bool,
) -> anyhow::Result<()> {
// TODO(anp): Remove after packaging a Windows stdio test server for Wine exec.
skip_if_wine_exec!(
@@ -1560,6 +1568,7 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
let server = responses::start_mock_server().await;
let call_id = "sandbox-meta-call";
let restricted_call_id = "owner-restricted-call";
let server_name = "rmcp";
let namespace = format!("mcp__{server_name}");
let mut models = codex_models_manager::bundled_models_response()?;
@@ -1572,15 +1581,20 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
model.node_repl_disabled = node_repl_disabled;
let models_mock = mount_models_once(&server, models).await;
let initial_mock = mount_sse_once(
&server,
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call_with_namespace(call_id, &namespace, "sandbox_meta", "{}"),
responses::ev_completed("resp-1"),
]),
)
.await;
let mut response_events = vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call_with_namespace(call_id, &namespace, "sandbox_meta", "{}"),
];
if attachment_owned_permissions {
response_events.push(responses::ev_function_call_with_namespace(
restricted_call_id,
&namespace,
"sync",
"{}",
));
}
response_events.push(responses::ev_completed("resp-1"));
let initial_mock = mount_sse_once(&server, responses::sse(response_events)).await;
let final_mock = mount_sse_once(
&server,
responses::sse(vec![
@@ -1609,6 +1623,91 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
.await?;
wait_for_mcp_server(&fixture.codex, server_name).await?;
let owner_permission_profile = if attachment_owned_permissions {
PermissionProfile::from_runtime_permissions(
&FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::Root,
},
FileSystemAccessMode::Write,
),
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
FileSystemAccessMode::Read,
),
FileSystemSandboxEntry::new(
FileSystemPath::Special {
value: FileSystemSpecialPath::project_roots(Some(".git".to_string())),
},
FileSystemAccessMode::Read,
),
]),
NetworkSandboxPolicy::Restricted,
)
} else {
PermissionProfile::read_only()
};
let owner_workspace_roots = if attachment_owned_permissions {
let selection = fixture
.codex
.environment_selections()
.await
.into_iter()
.find(|selection| selection.environment_id == remote_aware_environment_id())
.context("thread should select the MCP server's executor environment")?;
let selection = TurnEnvironmentSelection {
workspace_roots: vec![PathUri::parse(if cfg!(windows) {
"file:///foreign/workspace"
} else {
"file:///C:/workspace"
})?],
..selection
};
submit_thread_settings(
&fixture.codex,
ThreadSettingsOverrides {
environments: Some(TurnEnvironmentSelections::new(
fixture.config.cwd.clone(),
vec![TurnEnvironmentSelection {
config: EnvironmentConfigState::Pending,
..selection.clone()
}],
)),
..Default::default()
},
)
.await?;
let workspace_roots = selection.workspace_roots.clone();
fixture
.codex
.environment_ready(
&selection,
EnvironmentConfig {
allow_login_shell: fixture.config.permissions.allow_login_shell,
permission_profile: PermissionProfileSnapshot::legacy(
owner_permission_profile.clone(),
),
shell_environment_policy: Default::default(),
windows_sandbox_level: WindowsSandboxLevel::from_config(&fixture.config),
windows_sandbox_private_desktop: fixture
.config
.permissions
.windows_sandbox_private_desktop,
use_legacy_landlock: fixture.config.features.use_legacy_landlock(),
exec_policy: None,
mcp_policy: None,
network_policy: None,
selected_capability_roots: Vec::new(),
},
)
.await?;
workspace_roots
} else {
Vec::new()
};
fixture
.thread_manager
.get_models_manager()
@@ -1622,7 +1721,11 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
fixture
.submit_turn_with_permission_profile(
"call the rmcp sandbox_meta tool",
PermissionProfile::read_only(),
if attachment_owned_permissions {
PermissionProfile::Disabled
} else {
PermissionProfile::read_only()
},
)
.await?;
@@ -1641,7 +1744,19 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
json!(node_repl_disabled)
);
let output_item = final_mock.single_request().function_call_output(call_id);
let final_request = final_mock.single_request();
if attachment_owned_permissions {
let restricted_output = final_request
.function_call_output_text(restricted_call_id)
.expect("restricted MCP tool should produce a denied call output");
assert!(
restricted_output
.contains("MCP tool call requires approval, but approval policy is never"),
"attachment-owned permissions should deny the mutable MCP tool: {restricted_output}"
);
}
let output_item = final_request.function_call_output(call_id);
let output_text = output_item
.get("output")
.and_then(Value::as_str)
@@ -1665,10 +1780,26 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta(
.get(MCP_SANDBOX_STATE_META_CAPABILITY)
.expect("sandbox state metadata should be present");
let sandbox_state: SandboxState = serde_json::from_value(sandbox_meta.clone())?;
if attachment_owned_permissions {
let workspace_root = &owner_workspace_roots[0];
for path in [workspace_root.clone(), workspace_root.join(".git")?] {
let expected_entry =
FileSystemSandboxEntry::new(path.into(), FileSystemAccessMode::Read);
assert!(
sandbox_state
.permission_profile
.file_system_sandbox_policy()
.entries
.contains(&expected_entry),
"foreign workspace root should retain its owner restriction: {expected_entry:?}"
);
}
}
assert_eq!(
sandbox_state,
SandboxState {
permission_profile: PermissionProfile::read_only(),
permission_profile: owner_permission_profile
.materialize_project_roots_with_path_uris(&owner_workspace_roots),
codex_linux_sandbox_exe: fixture.config.codex_linux_sandbox_exe.clone(),
sandbox_cwd: PathUri::from_abs_path(&fixture.config.cwd),
use_legacy_landlock: false,
codex-rs/protocol/src/models.rs
+16 -3
View File
@@ -548,15 +548,28 @@ impl PermissionProfile {
pub fn materialize_project_roots_with_workspace_roots(
self,
workspace_roots: &[AbsolutePathBuf],
) -> Self {
self.materialize_project_roots_with(|policy| {
policy.materialize_project_roots_with_workspace_roots(workspace_roots)
})
}
pub fn materialize_project_roots_with_path_uris(self, workspace_roots: &[PathUri]) -> Self {
self.materialize_project_roots_with(|policy| {
policy.materialize_project_roots_with_path_uris(workspace_roots)
})
}
fn materialize_project_roots_with(
self,
materialize: impl FnOnce(FileSystemSandboxPolicy) -> FileSystemSandboxPolicy,
) -> Self {
match self {
Self::Managed {
file_system,
network,
} => {
let file_system = file_system
.to_sandbox_policy()
.materialize_project_roots_with_workspace_roots(workspace_roots);
let file_system = materialize(file_system.to_sandbox_policy());
Self::Managed {
file_system: ManagedFileSystemPermissions::from_sandbox_policy(&file_system),
network,
codex-rs/protocol/src/permissions.rs
+82
View File
@@ -7,6 +7,7 @@ use std::path::PathBuf;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_absolute_path::canonicalize_preserving_symlinks;
use codex_utils_path_uri::LegacyAppPathString;
use codex_utils_path_uri::PathConvention;
use codex_utils_path_uri::PathUri;
use globset::GlobBuilder;
use globset::GlobMatcher;
@@ -1030,6 +1031,65 @@ impl FileSystemSandboxPolicy {
self
}
/// Materializes workspace-root entries without projecting executor paths onto the host.
pub fn materialize_project_roots_with_path_uris(mut self, workspace_roots: &[PathUri]) -> Self {
if let Ok(native_workspace_roots) = workspace_roots
.iter()
.map(PathUri::to_abs_path)
.collect::<Result<Vec<_>, _>>()
{
return self.materialize_project_roots_with_workspace_roots(&native_workspace_roots);
}
let mut entries = Vec::with_capacity(self.entries.len());
for entry in self.entries {
let (subpath, is_glob) = match &entry.path {
FileSystemPath::Special {
value: FileSystemSpecialPath::ProjectRoots { subpath },
} => (subpath.as_deref(), false),
FileSystemPath::GlobPattern { pattern }
if pattern.starts_with(PROJECT_ROOTS_GLOB_PATTERN_PREFIX) =>
{
(
Some(&pattern[PROJECT_ROOTS_GLOB_PATTERN_PREFIX.len()..]),
true,
)
}
_ => {
entries.push(entry);
continue;
}
};
entries.extend(workspace_roots.iter().filter_map(|root| {
let path = subpath.map_or_else(
|| Some(root.clone()),
|subpath| resolve_scoped_workspace_path(root, subpath),
);
let (path, access) = match path {
Some(path) if is_glob => (
FileSystemPath::GlobPattern {
pattern: path.inferred_native_path_string(),
},
entry.access,
),
Some(path) => (FileSystemPath::Path { path }, entry.access),
None if !entry.access.can_write() => (
FileSystemPath::Path { path: root.clone() },
FileSystemAccessMode::Deny,
),
None => return None,
};
Some(FileSystemSandboxEntry {
path,
access,
missing_path_behavior: entry.missing_path_behavior,
})
}));
}
self.entries = entries;
self
}
/// Preserves symbolic `:workspace_roots` entries while also adding concrete
/// entries for each provided workspace root.
pub fn with_materialized_project_roots_for_workspace_roots(
@@ -1600,6 +1660,28 @@ fn resolve_candidate_path(path: &Path, cwd: &Path) -> Option<AbsolutePathBuf> {
}
}
/// Resolves a workspace-relative path using the root's own path convention.
///
/// Rejects absolute paths, traversal, Windows drive changes, and any result
/// outside the root so foreign-platform permission rules cannot escape scope.
fn resolve_scoped_workspace_path(root: &PathUri, subpath: &str) -> Option<PathUri> {
let convention = root.infer_path_convention()?;
if subpath.starts_with('/')
|| convention == PathConvention::Windows && subpath.starts_with('\\')
|| convention
.path_segments(subpath)
.any(|segment| segment == "." || segment == "..")
|| convention == PathConvention::Windows
&& convention
.path_segments(subpath)
.any(|segment| segment.contains(':'))
{
return None;
}
let path = root.join(subpath).ok()?;
path.starts_with(root).then_some(path)
}
/// Returns true when two config paths refer to the same exact target before
/// any prefix matching is applied.
///