mirror of
https://github.com/openai/codex.git
synced 2026-10-22 17:58:46 +00:00
Preserve enterprise MCP authorization across runtime refreshes (#55696)
## Why Configuration reloads must cancel stale enterprise MCP startups while preserving access for running threads that still pass validation. Replacing a runtime during `thread/revert` must also leave the replacement able to use enterprise tools. ## What changed - Separate pending startup cancellation from published thread lifetimes, with independent startup state for forks and replacement runtimes. - Retain eligible enterprise MCP connections across refreshes and explicit reconnects while applying updated tool restrictions and approvals. Do not create replacement enterprise connections after initial startup. - Revoke enterprise authorization when a thread is removed, drain requests when access is disabled, and revoke a connection's authority when token renewal requires authentication. ## Testing Extend regression coverage to verify enterprise tool calls survive `thread/revert`, inherited fork configurations are cancelled after a reload, and enterprise calls fail after logout while ordinary MCP calls remain available. GitOrigin-RevId: 2d634be3011854b57f73c741bd1ec6b5aed94ee5
This commit is contained in:
@@ -57,7 +57,13 @@ pub(crate) struct ConfigManager {
|
||||
network_policy_reload: Arc<tokio::sync::Semaphore>,
|
||||
network_policy_snapshot: Arc<RwLock<Option<Arc<ApplicationPolicySnapshot>>>>,
|
||||
/// Captured before config loading so a reload also revokes pending startups.
|
||||
mcp_ema_cancellation: Arc<Mutex<CancellationToken>>,
|
||||
mcp_ema_cancellation: Arc<Mutex<EmaCancellationRoots>>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct EmaCancellationRoots {
|
||||
startup: CancellationToken,
|
||||
lifetime: CancellationToken,
|
||||
}
|
||||
|
||||
/// Startup configuration, including whether loading had to fall back to defaults.
|
||||
@@ -117,22 +123,33 @@ impl ConfigManager {
|
||||
self.codex_home.as_path()
|
||||
}
|
||||
|
||||
/// Retires EMA access for every config already loaded or being loaded.
|
||||
/// New loads resolve their own settings under the next cancellation scope.
|
||||
/// Explicit account, policy-owner, and enterprise-login changes revoke cached access.
|
||||
pub(crate) fn revoke_mcp_ema(&self) {
|
||||
let mut current = self
|
||||
.mcp_ema_cancellation
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
current.cancel();
|
||||
*current = CancellationToken::new();
|
||||
current.startup.cancel();
|
||||
current.lifetime.cancel();
|
||||
*current = EmaCancellationRoots::default();
|
||||
}
|
||||
|
||||
fn capture_mcp_ema_cancellation(&self) -> CancellationToken {
|
||||
self.mcp_ema_cancellation
|
||||
/// Published threads are validated individually by the existing refresh loop.
|
||||
pub(crate) fn revoke_mcp_ema_startups(&self) {
|
||||
let mut current = self
|
||||
.mcp_ema_cancellation
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner)
|
||||
.child_token()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
current.startup.cancel();
|
||||
current.startup = CancellationToken::new();
|
||||
}
|
||||
|
||||
fn capture_mcp_ema_cancellation(&self) -> codex_config::McpEmaCancellation {
|
||||
let current = self
|
||||
.mcp_ema_cancellation
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
codex_config::McpEmaCancellation::pending(current.startup.clone(), current.lifetime.clone())
|
||||
}
|
||||
|
||||
pub(crate) fn with_embedded_network_policy(
|
||||
|
||||
@@ -46,7 +46,7 @@ async fn reload_mcp_config_with_policy(
|
||||
config_manager: &ConfigManager,
|
||||
policy: ReloadPolicy,
|
||||
) -> io::Result<()> {
|
||||
config_manager.revoke_mcp_ema();
|
||||
config_manager.revoke_mcp_ema_startups();
|
||||
let mut rejected = false;
|
||||
let mut load_error = None;
|
||||
let mut completed: Vec<Arc<CodexThread>> = Vec::new();
|
||||
@@ -416,6 +416,14 @@ enabled = false
|
||||
.mcp_ema_cancellation()
|
||||
.is_cancelled()
|
||||
);
|
||||
// Prediction forks inherit a published config and may outlive their parent.
|
||||
let parent = thread_manager
|
||||
.start_thread(codex_core::StartThreadOptions::new(pending_startup.clone()))
|
||||
.await?;
|
||||
let (inherited_fork, _) = thread_manager
|
||||
.fork_options_from_parent(parent.thread_id)
|
||||
.await?;
|
||||
thread_manager.remove_thread(&parent.thread_id).await;
|
||||
std::fs::write(
|
||||
temp_dir.path().join(codex_config::CONFIG_TOML_FILE),
|
||||
r#"
|
||||
@@ -427,6 +435,17 @@ enabled = false
|
||||
|
||||
reload_mcp_config(&thread_manager, &config_manager).await?;
|
||||
|
||||
let fork = thread_manager.start_thread(inherited_fork).await?;
|
||||
assert!(
|
||||
fork.thread
|
||||
.config()
|
||||
.await
|
||||
.config_layer_stack
|
||||
.mcp_ema_cancellation()
|
||||
.is_cancelled()
|
||||
);
|
||||
fork.thread.shutdown_and_wait().await?;
|
||||
parent.thread.shutdown_and_wait().await?;
|
||||
assert!(
|
||||
pending_startup
|
||||
.config_layer_stack
|
||||
|
||||
@@ -374,7 +374,7 @@ pub(super) async fn reload_user_config(
|
||||
}
|
||||
|
||||
async fn reload_user_config_inner(config_manager: &ConfigManager, thread_manager: &ThreadManager) {
|
||||
config_manager.revoke_mcp_ema();
|
||||
config_manager.revoke_mcp_ema_startups();
|
||||
if let Err(err) = config_manager.load_config_layers(/*cwd*/ None).await {
|
||||
tracing::warn!("failed to rebuild user config for runtime refresh: {err}");
|
||||
for thread_id in thread_manager.list_thread_ids().await {
|
||||
|
||||
@@ -2211,8 +2211,17 @@ impl ThreadRequestProcessor {
|
||||
"thread/revert only supports paginated threads",
|
||||
));
|
||||
}
|
||||
let mut config = thread.config().await.as_ref().clone();
|
||||
// Give the replacement its own pending lifetime before removal revokes the old runtime.
|
||||
let ema_cancellation = config
|
||||
.config_layer_stack
|
||||
.mcp_ema_cancellation()
|
||||
.for_startup();
|
||||
config.config_layer_stack = config
|
||||
.config_layer_stack
|
||||
.with_mcp_ema_cancellation(ema_cancellation);
|
||||
let runtime_snapshot = ThreadRevertRuntimeSnapshot {
|
||||
config: thread.config().await.as_ref().clone(),
|
||||
config,
|
||||
settings: thread.restorable_thread_settings().await,
|
||||
client_mcp_extensions: thread.client_mcp_extensions(),
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user