From 374c4b2d828321cfa3d3fc69ce66870f8b7fd01f Mon Sep 17 00:00:00 2001 From: Nick Steele Date: Mon, 14 Sep 2026 16:20:14 +0000 Subject: [PATCH] Resolve enterprise-managed MCP registrations in the catalog (#45459) ## What changed - Retain the trusted enterprise identity provider in runtime configuration and bind winning MCP registrations during catalog finalization. Require `features.use_xaa` and a configured identity provider for activation, while preserving existing server restrictions. - Apply plugin `ema_auth` client, issuer, resource, and scope settings to installed and selected plugins. Disable registrations with mismatched endpoints or empty resources without rewriting plugin endpoints. - Preserve enterprise auth policy across catalog rebuilds and rebind registrations when materialized server settings change. Keep registration rejection separate from persistent server-name vetoes so it does not disable replacement hosted apps. ## Testing Add coverage for activation gates, configuration ownership, plugin endpoint validation, catalog rebuilds, and skipping interactive OAuth during installation of enterprise-managed plugins. Stabilize the sandbox network proxy test by reading request headers before closing the loopback connection. GitOrigin-RevId: 3374f507d120835b285767cedbbb511fc7b0fba2 --- .../tests/suite/v2/plugin_install.rs | 26 ++- codex-rs/cli/tests/sandbox_network_proxy.rs | 14 ++ codex-rs/codex-mcp/src/catalog.rs | 47 ++++- codex-rs/codex-mcp/src/catalog_tests.rs | 136 ++++++++++++ codex-rs/codex-mcp/src/mcp/mod.rs | 4 + codex-rs/codex-mcp/src/mcp/mod_tests.rs | 59 ++++++ codex-rs/config/src/mcp_ema.rs | 31 +++ codex-rs/config/src/mcp_ema_tests.rs | 196 ++++++++++++++++++ codex-rs/config/src/mcp_types.rs | 10 + codex-rs/config/src/types.rs | 28 +++ codex-rs/core-plugins/src/loader.rs | 10 +- codex-rs/core-plugins/src/loader_tests.rs | 96 +++++++++ codex-rs/core/src/config/mod.rs | 70 ++++--- codex-rs/core/tests/suite/mcp_ema_config.rs | 111 +++++++++- codex-rs/thread-manager-sample/src/main.rs | 1 + 15 files changed, 792 insertions(+), 47 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index 1e4adf9034..84a62aa9cb 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -1492,18 +1492,30 @@ url = "https://example.com/allowed-mcp" Ok(()) } +#[test_case("disabled")] +#[test_case("enterprise")] #[tokio::test] -async fn plugin_install_skips_mcp_oauth_disabled_by_plugin_config() -> Result<()> { +async fn plugin_install_skips_mcp_oauth_managed_by_plugin_config(case: &str) -> Result<()> { let oauth_server = MockServer::start().await; + let endpoint = format!("{}/mcp", oauth_server.uri()); + let mcp_settings = match case { + "disabled" => "enabled = false".to_string(), + "enterprise" => format!( + "ema_auth = {{ url = '{endpoint}', resource = '{endpoint}', client_id = 'resource-client', authorization_server_issuer = 'https://as.example' }}" + ), + _ => unreachable!("unknown test case"), + }; let codex_home = TempDir::new()?; std::fs::write( codex_home.path().join("config.toml"), - r#"[features] + format!( + r#"[features] plugins = true [plugins."sample-plugin@debug".mcp_servers.sample-mcp] -enabled = false -"#, +{mcp_settings} +"# + ), )?; let repo_root = TempDir::new()?; @@ -1550,10 +1562,8 @@ enabled = false .get("plugins") .and_then(|plugins| plugins.get("sample-plugin@debug")) .and_then(|plugin| plugin.get("mcp_servers")) - .and_then(|servers| servers.get("sample-mcp")) - .and_then(|server| server.get("enabled")) - .and_then(toml::Value::as_bool), - Some(false) + .and_then(|servers| servers.get("sample-mcp")), + Some(&toml::from_str::(&mcp_settings)?) ); Ok(()) } diff --git a/codex-rs/cli/tests/sandbox_network_proxy.rs b/codex-rs/cli/tests/sandbox_network_proxy.rs index c029b67215..21747c3919 100644 --- a/codex-rs/cli/tests/sandbox_network_proxy.rs +++ b/codex-rs/cli/tests/sandbox_network_proxy.rs @@ -82,6 +82,20 @@ fn sandbox_with_network_proxy_allows_explicit_loopback_access() -> Result<()> { loop { match listener.accept() { Ok((mut stream, _)) => { + // Closing with unread request bytes can reset the connection and yield a 502. + stream.set_nonblocking(false)?; + stream.set_read_timeout(Some(Duration::from_secs(5)))?; + let mut request = Vec::new(); + let mut buffer = [0_u8; 1024]; + while !request.windows(4).any(|window| window == b"\r\n\r\n") { + let bytes_read = std::io::Read::read(&mut stream, &mut buffer)?; + if bytes_read == 0 || request.len() + bytes_read > 64 * 1024 { + return Err(std::io::Error::other( + "incomplete or oversized loopback request headers", + )); + } + request.extend_from_slice(&buffer[..bytes_read]); + } std::io::Write::write_all( &mut stream, b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n", diff --git a/codex-rs/codex-mcp/src/catalog.rs b/codex-rs/codex-mcp/src/catalog.rs index 14189c8ab3..486b7a9480 100644 --- a/codex-rs/codex-mcp/src/catalog.rs +++ b/codex-rs/codex-mcp/src/catalog.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use codex_config::McpServerConfig; use codex_config::McpServerDisabledReason; +use codex_config::McpServerIdpOAuthConfig; use codex_config::RequirementSource; use codex_protocol::mcp_policy::EnvironmentMcpPolicy; use codex_utils_path_uri::PathUri; @@ -320,9 +321,16 @@ impl CatalogAction { pub struct McpCatalogBuilder { actions: Vec, disabled_server_names: BTreeSet, + ema_idp: Option, } impl McpCatalogBuilder { + /// Enables EMA with the IdP selected from trusted configuration. + /// Without this policy, finalization disables EMA registrations. + pub fn enable_ema(&mut self, idp: McpServerIdpOAuthConfig) { + self.ema_idp = Some(idp); + } + pub fn register(&mut self, registration: McpServerRegistration) { self.actions .push(CatalogAction::Register(Box::new(registration))); @@ -429,6 +437,14 @@ impl McpCatalogBuilder { } pub fn build(mut self) -> ResolvedMcpCatalog { + // Keep source actions unbound so later catalog revisions resolve afresh. + for action in &mut self.actions { + if let CatalogAction::Register(registration) = action + && let Some(oauth) = &mut registration.config.oauth + { + oauth.ema_registration = None; + } + } // Stable sorting makes action order the tie-breaker when precedence is equal. self.actions.sort_by_key(CatalogAction::precedence); @@ -461,13 +477,18 @@ impl McpCatalogBuilder { } let mut disabled_server_names = self.disabled_server_names; + let ema_idp = self.ema_idp; let servers = winners .into_iter() .filter_map(|(name, action)| match action { CatalogAction::Register(registration) => { let mut registration = *registration; let persist_disabled_name = - registration.source.disabled_registration_is_name_veto(); + registration.source.disabled_registration_is_name_veto() + && !matches!( + registration.config.disabled_reason, + Some(McpServerDisabledReason::EmaRegistration) + ); if !registration.config.enabled || disabled_server_names.contains(&name) { registration.config.enabled = false; if persist_disabled_name { @@ -475,6 +496,16 @@ impl McpCatalogBuilder { disabled_server_names.insert(name.clone()); } } + if matches!( + registration.config.auth, + codex_config::McpServerAuth::EmaAuth + ) { + let allowed = ema_idp.as_ref().is_some_and(|idp| { + registration.config.resolve_ema_registration(idp).is_ok() + }); + // EMA denial must not become a persistent name veto. + registration.config.enabled &= allowed; + } Some(( name, ResolvedMcpServer { @@ -491,6 +522,7 @@ impl McpCatalogBuilder { ResolvedMcpCatalog { actions: self.actions, disabled_server_names, + ema_idp, servers, conflicts, } @@ -524,6 +556,7 @@ impl ResolvedMcpServer { pub struct ResolvedMcpCatalog { actions: Vec, disabled_server_names: BTreeSet, + ema_idp: Option, servers: BTreeMap, conflicts: Vec, } @@ -537,6 +570,7 @@ impl ResolvedMcpCatalog { McpCatalogBuilder { actions: self.actions.clone(), disabled_server_names: self.disabled_server_names.clone(), + ema_idp: self.ema_idp.clone(), } } @@ -551,16 +585,19 @@ impl ResolvedMcpCatalog { .collect() } - /// Returns whether both catalogs resolve to the same winning servers and sources. + /// Returns whether both catalogs have the same winning servers, sources, and EMA policy. pub fn has_same_servers(&self, other: &Self) -> bool { - self.servers == other.servers + self.servers == other.servers && self.ema_idp == other.ema_idp } - /// Replaces the resolved server set while preserving known server sources. + /// Replaces the resolved server set while preserving known sources and EMA policy. /// /// Names not present in the existing catalog are treated as config-owned. pub fn with_materialized_servers(&self, servers: HashMap) -> Self { - let mut builder = Self::builder(); + let mut builder = McpCatalogBuilder { + ema_idp: self.ema_idp.clone(), + ..Default::default() + }; for (name, config) in servers { let previous = self.server(&name); let source = previous diff --git a/codex-rs/codex-mcp/src/catalog_tests.rs b/codex-rs/codex-mcp/src/catalog_tests.rs index 39f4d11008..4c5d1ed0cd 100644 --- a/codex-rs/codex-mcp/src/catalog_tests.rs +++ b/codex-rs/codex-mcp/src/catalog_tests.rs @@ -4,9 +4,13 @@ use std::time::Duration; use codex_config::AppToolApproval; use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; +use codex_config::McpServerAuth; use codex_config::McpServerConfig; +use codex_config::McpServerDisabledReason; +use codex_config::McpServerIdpOAuthConfig; use codex_config::McpServerToolConfig; use codex_config::McpServerTransportConfig; +use codex_config::types::PluginMcpServerEmaAuthConfig; use codex_protocol::mcp_policy::EnvironmentMcpPolicy; use codex_protocol::mcp_policy::PluginMcpRequirements; use codex_utils_path_uri::PathUri; @@ -116,6 +120,138 @@ fn plugin_host_root_is_retained_in_catalog_identity() { assert!(!original.has_same_servers(&replacement)); } +#[test] +fn ema_policy_survives_rebuilds_and_rebinds_materialized_servers() { + let idp = McpServerIdpOAuthConfig { + issuer: "https://idp.example".to_string(), + client_id: "enterprise-client".to_string(), + }; + let mut builder = ResolvedMcpCatalog::builder(); + builder.enable_ema(idp.clone()); + let enabled_empty = builder.build(); + assert!(!enabled_empty.has_same_servers(&ResolvedMcpCatalog::default())); + + let mut builder = enabled_empty.to_builder(); + let mut config = server("https://resource.example/mcp"); + config.auth = McpServerAuth::EmaAuth; + builder.register(McpServerRegistration::from_config( + "enterprise".to_string(), + config, + )); + let original = builder.build(); + assert!(original.has_same_servers(&original.to_builder().build())); + + let mut servers = original.configured_servers(); + let config = servers.get_mut("enterprise").expect("EMA server"); + let McpServerTransportConfig::StreamableHttp { url, .. } = &mut config.transport else { + panic!("expected HTTP server"); + }; + *url = "https://resource.example/revised".to_string(); + let materialized = original.with_materialized_servers(servers); + let config = materialized + .server("enterprise") + .expect("materialized EMA server") + .config(); + let registration = config + .ema_registration() + .expect("finalized EMA registration"); + assert_eq!( + ( + config.enabled, + registration.idp(), + registration.server_url() + ), + (true, &idp, "https://resource.example/revised") + ); + + let mut builder = ResolvedMcpCatalog::builder(); + builder.register(McpServerRegistration::from_config( + "enterprise".to_string(), + config.clone(), + )); + let denied = builder.build(); + let config = denied + .server("enterprise") + .expect("denied EMA server") + .config(); + assert_eq!((config.enabled, config.ema_registration()), (false, None)); +} + +#[test] +fn rejected_plugin_ema_registration_does_not_veto_hosted_apps() { + let idp = McpServerIdpOAuthConfig { + issuer: "https://idp.example".to_string(), + client_id: "enterprise-client".to_string(), + }; + for (url, resource) in [ + ("https://other.example/mcp", "https://resource.example"), + ("https://resource.example/mcp", " "), + ] { + for initially_enabled in [true, false] { + let mut rejected = server(url); + rejected.enabled = initially_enabled; + let policy = PluginMcpServerEmaAuthConfig { + url: "https://resource.example/mcp".to_string(), + client_id: "resource-client".to_string(), + authorization_server_issuer: "https://as.example".to_string(), + scopes: Vec::new(), + resource: resource.to_string(), + }; + policy.apply(&mut rejected); + assert!(rejected.resolve_ema_registration(&idp).is_err()); + assert_eq!( + ( + rejected.enabled, + rejected.auth.clone(), + rejected.ema_registration() + ), + (false, McpServerAuth::EmaAuth, None), + ); + assert_eq!( + rejected.disabled_reason, + initially_enabled.then_some(McpServerDisabledReason::EmaRegistration), + ); + + let mut builder = ResolvedMcpCatalog::builder(); + builder.enable_ema(idp.clone()); + builder.register(McpServerRegistration::from_plugin( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + plugin("plugin@test"), + /*plugin_order*/ 0, + rejected.clone(), + )); + let catalog = builder.build(); + assert_eq!( + catalog.server(CODEX_APPS_MCP_SERVER_NAME).unwrap().config(), + &rejected, + ); + + let materialized = catalog.with_materialized_servers(catalog.configured_servers()); + for catalog in [catalog, materialized] { + let mut builder = catalog.to_builder(); + let mut expected = server("https://chatgpt.com/mcp"); + builder.register(McpServerRegistration::from_hosted_apps( + "apps", + /*contribution_order*/ 0, + expected.clone(), + )); + expected.enabled = initially_enabled; + assert_eq!( + builder.build().server(CODEX_APPS_MCP_SERVER_NAME), + Some(&ResolvedMcpServer { + source: McpServerSource::Extension { + id: "apps".to_string(), + host_owned_apps: true, + }, + config: expected, + protocol_mode: None, + }), + ); + } + } + } +} + #[test] fn source_precedence_preserves_the_winning_registration() { let extension = server("https://extension.example/mcp"); diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 180a1efb5f..43d9a43c0b 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -21,6 +21,7 @@ use std::time::Duration; use codex_config::ConfigLayerStack; use codex_config::Constrained; +use codex_config::McpEnterpriseManagedAuthConfig; use codex_config::McpServerAuth; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; @@ -127,6 +128,9 @@ pub struct McpConfig { pub apps_mcp_product_sku: Option, /// Codex home directory used for MCP OAuth state and app-tool cache files. pub codex_home: PathBuf, + /// Trusted enterprise IdP inherited after normal catalog and policy resolution. + pub mcp_enterprise_managed_auth: Option, + pub xaa_enabled: bool, /// Preferred credential store for MCP OAuth tokens. pub mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode, /// OAuth refresh ownership selected for new MCP connections. diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 6272a57ce7..e45d6bcacc 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -87,6 +87,8 @@ pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig { chatgpt_base_url: "https://chatgpt.com".to_string(), apps_mcp_product_sku: None, codex_home, + mcp_enterprise_managed_auth: None, + xaa_enabled: false, mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::default(), oauth_refresh_mode: McpOAuthRefreshMode::Legacy, auth_keyring_backend_kind: AuthKeyringBackendKind::default(), @@ -127,6 +129,63 @@ pub(crate) fn test_elicitation_config( Arc::new(config) } +#[test] +fn ema_catalog_supports_configured_installed_and_selected_plugins_without_widening_policy() { + let server: McpServerConfig = serde_json::from_value(serde_json::json!({ + "url": "https://resource.example/mcp", "auth": "ema_auth", + "oauth": { "client_id": "resource-client", "authorization_server_issuer": "https://as.example" } + })).unwrap(); + let plugin = McpPluginAttribution::agent_plugin("plugin@test".into(), "Plugin".into()); + let mut catalog = ResolvedMcpCatalog::builder(); + catalog.register(McpServerRegistration::from_config( + "configured".into(), + server.clone(), + )); + catalog.register(McpServerRegistration::from_plugin( + "installed".into(), + plugin.clone(), + /*plugin_order*/ 0, + server.clone(), + )); + catalog.register(McpServerRegistration::from_selected_plugin( + "selected".into(), + plugin, + /*selection_order*/ 0, + server, + )); + let mut config = test_mcp_config(PathBuf::new()); + let idp = codex_config::McpServerIdpOAuthConfig { + issuer: "https://idp.example".into(), + client_id: "enterprise-client".into(), + }; + let deny_all = codex_protocol::mcp_policy::EnvironmentMcpPolicy { + servers: Some(Default::default()), + plugins: None, + }; + for (xaa_enabled, denied) in [(true, false), (true, true), (false, false)] { + let mut catalog = catalog.clone(); + if xaa_enabled { + catalog.enable_ema(idp.clone()); + } + config.mcp_server_catalog = catalog.build_with_environment_authority(|_| { + if denied { + crate::McpEnvironmentAuthority::Restricted(&deny_all) + } else { + crate::McpEnvironmentAuthority::Unrestricted + } + }); + let servers = effective_mcp_servers(&config, /*auth*/ None); + for name in ["configured", "installed", "selected"] { + assert_eq!(servers[name].enabled(), xaa_enabled && !denied, "{name}"); + assert_eq!( + servers[name].config().oauth_idp(), + xaa_enabled.then_some(&idp) + ); + assert_eq!(servers[name].config().auth, McpServerAuth::EmaAuth); + } + } +} + #[test] fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() { assert_eq!( diff --git a/codex-rs/config/src/mcp_ema.rs b/codex-rs/config/src/mcp_ema.rs index 6c5e4a863e..8360f22ebc 100644 --- a/codex-rs/config/src/mcp_ema.rs +++ b/codex-rs/config/src/mcp_ema.rs @@ -139,6 +139,37 @@ impl McpServerConfig { self.oauth.as_ref()?.ema_registration.as_ref() } + /// Called once on a materialized catalog, after configuration provenance and + /// plugin endpoint policy have been checked. Runtime consumers use the result. + pub fn resolve_ema_registration( + &mut self, + idp: &McpServerIdpOAuthConfig, + ) -> Result<(), &'static str> { + if let Some(oauth) = &mut self.oauth { + oauth.ema_registration = None; + if let Some(error) = oauth.ema_registration_error { + return Err(error); + } + } + if !matches!(self.auth, crate::McpServerAuth::EmaAuth) { + return Err("enterprise registration requires ema_auth"); + } + self.validate_ema_auth_transport()?; + let McpServerTransportConfig::StreamableHttp { url, .. } = &self.transport else { + unreachable!("EMA transport was validated"); + }; + let oauth = self.oauth.get_or_insert_default(); + oauth.ema_registration = Some(McpEmaRegistration { + idp: idp.clone(), + server_url: url.clone(), + resource: self.oauth_resource.clone(), + client_id: oauth.client_id.clone(), + authorization_server_issuer: oauth.authorization_server_issuer.clone(), + scopes: self.scopes.clone().unwrap_or_default(), + }); + Ok(()) + } + /// EMA never falls back to an unrelated bearer or executor-owned credential. pub fn validate_ema_auth_transport(&self) -> Result<(), &'static str> { if !self.is_local_environment() { diff --git a/codex-rs/config/src/mcp_ema_tests.rs b/codex-rs/config/src/mcp_ema_tests.rs index 5b3f50eed7..5211497725 100644 --- a/codex-rs/config/src/mcp_ema_tests.rs +++ b/codex-rs/config/src/mcp_ema_tests.rs @@ -33,6 +33,21 @@ fn local_sources(name: &str) -> (ConfigLayerSource, ConfigLayerSource, ConfigLay ) } +fn validate_effective_ema(stack: &ConfigLayerStack) -> std::io::Result<()> { + let servers = stack + .effective_config() + .get("mcp_servers") + .cloned() + .unwrap_or_else(|| toml::Value::Table(Default::default())) + .try_into::>() + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidInput, error))?; + validate_ema_auth_sources(stack, &servers) +} + +fn valid_ema_layers(layers: Vec<(ConfigLayerSource, &str)>) -> bool { + validate_effective_ema(&stack(layers)).is_ok() +} + #[test] fn ema_profiles_and_auth_modes_preserve_non_project_authority() { let (system, user, project) = local_sources("ema-config"); @@ -91,6 +106,187 @@ fn ema_profiles_and_auth_modes_preserve_non_project_authority() { assert_eq!(server.oauth_idp(), None); } +#[test] +fn ema_registrations_require_one_atomic_non_project_source() { + let (system, user, project) = local_sources("ema-registration-sources"); + for (section, authorization, protected_changes) in [ + ( + "mcp_servers.enterprise", + r#" + auth='ema_auth' + oauth_resource='https://resource.example' + oauth.client_id='resource-client' + oauth.authorization_server_issuer='https://as.example' + "#, + &[ + "oauth_resource='https://other.example'", + "oauth.client_id='other-client'", + "oauth.authorization_server_issuer='https://other-as.example'", + ] as &[&str], + ), + ( + "plugins.\"sample@test\".mcp_servers.enterprise.ema_auth", + r#" + resource='https://resource.example' + client_id='resource-client' + authorization_server_issuer='https://as.example' + "#, + &[ + "resource='https://other.example'", + "client_id='other-client'", + "authorization_server_issuer='https://other-as.example'", + ], + ), + ] { + let registration = format!( + "[{section}]\nurl='https://resource.example/mcp'\nscopes=['tools']\n{authorization}" + ); + for (source, allowed) in [ + (system.clone(), true), + (user.clone(), true), + (project.clone(), false), + ] { + assert_eq!( + valid_ema_layers(vec![(source, registration.as_str())]), + allowed, + "{section}" + ); + } + for change in protected_changes.iter().copied().chain([ + "url='https://other.example/mcp'", + "url='https://resource.example/mcp/other'", + "scopes=['admin']", + ]) { + let overlay = format!("[{section}]\n{change}"); + assert!( + !valid_ema_layers(vec![ + (system.clone(), ®istration), + (project.clone(), &overlay) + ]), + "{section}: {change}" + ); + } + let higher = registration.replace("tools", "managed-tools"); + assert!( + !valid_ema_layers(vec![ + (system.clone(), ®istration), + (user.clone(), &higher), + (project.clone(), ®istration) + ]), + "project rollback: {section}" + ); + let policy_section = section.strip_suffix(".ema_auth").unwrap_or(section); + let policy = format!( + "[{policy_section}]\nenabled=false\n[{policy_section}.tools.read]\napproval_mode='prompt'" + ); + assert!(valid_ema_layers(vec![ + (system.clone(), ®istration), + (project.clone(), &policy) + ])); + if section.ends_with(".ema_auth") { + for required in [ + "url='https://resource.example/mcp'\n", + "resource='https://resource.example'\n", + ] { + assert!( + !valid_ema_layers(vec![(system.clone(), ®istration.replace(required, ""))]), + "missing {required}" + ); + } + } else { + assert!(valid_ema_layers(vec![ + (system.clone(), ®istration), + (project.clone(), "[mcp_servers.enterprise]\nauth='ema_auth'") + ])); + } + } +} + +#[test] +fn project_cannot_reenable_trusted_disabled_ema_registration() { + let (system, user, project) = local_sources("ema-disabled-source"); + let trusted = "[mcp_servers.enterprise]\nurl='https://resource.example/mcp'\nauth='ema_auth'\nenabled=false"; + let project_enable = "[mcp_servers.enterprise]\nenabled=true"; + assert!(!valid_ema_layers(vec![ + (system.clone(), trusted), + (project.clone(), project_enable) + ])); + assert!(valid_ema_layers(vec![ + (system.clone(), trusted), + (user, project_enable) + ])); + let enabled = trusted.replace("enabled=false", "enabled=true"); + assert!(valid_ema_layers(vec![ + (system, &enabled), + (project, "[mcp_servers.enterprise]\nenabled=false") + ])); +} + +#[test] +fn project_cannot_reenable_trusted_disabled_ema_plugin() { + let (system, _, project) = local_sources("ema-disabled-plugin"); + let trusted = r#" + [plugins."sample@test".mcp_servers.enterprise] + enabled=false + [plugins."sample@test".mcp_servers.enterprise.ema_auth] + url='https://resource.example/mcp' + client_id='resource-client' + authorization_server_issuer='https://as.example' + resource='https://resource.example' + "#; + let project_enable = "[plugins.\"sample@test\".mcp_servers.enterprise]\nenabled=true"; + assert!(!valid_ema_layers(vec![ + (system.clone(), trusted), + (project.clone(), project_enable) + ])); + assert!(valid_ema_layers(vec![ + (system, trusted), + ( + project, + "[plugins.\"sample@test\".mcp_servers.enterprise]\nenabled=false" + ) + ])); +} + +#[test] +fn non_project_auth_changes_and_ordinary_oauth_remain_allowed() { + let (system, user, project) = local_sources("ema-auth-downgrade"); + for (base_auth, source) in [("ema_auth", user), ("oauth", project)] { + let base = format!( + "[mcp_servers.enterprise]\nurl='https://resource.example/mcp'\nauth='{base_auth}'" + ); + assert!( + validate_effective_ema(&stack(vec![ + (system.clone(), &base), + ( + source, + "[mcp_servers.enterprise]\nauth='oauth'\nenabled=false" + ), + ])) + .is_ok() + ); + } +} + +#[test] +fn xaa_opt_in_requires_a_non_project_source() { + let (_, user, project) = local_sources("xaa-sources"); + let session = ConfigLayerSource::SessionFlags; + let enabled = "[features]\nuse_xaa=true"; + + for (source, allowed) in [(user, true), (session, true), (project.clone(), false)] { + assert_eq!( + validate_xaa_opt_in_source(&stack(vec![(source, enabled)]), /*xaa_enabled*/ true,) + .is_ok(), + allowed + ); + } + assert!( + validate_xaa_opt_in_source(&stack(vec![(project, enabled)]), /*xaa_enabled*/ false,) + .is_ok() + ); +} + #[test] fn ema_rejects_alternate_credentials_and_executor_custody() { for extra in [ diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index 38d559df51..6cc30861de 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -60,6 +60,8 @@ pub enum McpServerDisabledReason { Unknown, /// The server was disabled by config requirements from the given source. Requirements { source: RequirementSource }, + /// Enterprise authorization was rejected for this registration, not its name. + EmaRegistration, } impl fmt::Display for McpServerDisabledReason { @@ -69,6 +71,9 @@ impl fmt::Display for McpServerDisabledReason { McpServerDisabledReason::Requirements { source } => { write!(f, "requirements ({source})") } + McpServerDisabledReason::EmaRegistration => { + write!(f, "invalid enterprise registration") + } } } } @@ -175,6 +180,11 @@ pub struct McpServerOAuthConfig { #[serde(skip)] #[schemars(skip)] pub ema_registration: Option, + + /// Host-policy rejection retained until catalog finalization; never deserialized. + #[serde(skip)] + #[schemars(skip)] + pub ema_registration_error: Option<&'static str>, } /// Authentication flow for an HTTP MCP server. Explicit credentials take diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 6fc4b09146..709c953990 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -981,6 +981,34 @@ pub struct PluginMcpServerEmaAuthConfig { pub resource: String, } +impl PluginMcpServerEmaAuthConfig { + pub fn apply(&self, server: &mut McpServerConfig) { + let registration_error = if self.resource.trim().is_empty() { + Some("plugin EMA registration requires a resource") + } else if !server.matches_requirement(&crate::McpServerRequirement::Identity { + identity: crate::McpServerIdentity::Url { + url: self.url.clone(), + }, + }) { + Some("plugin endpoint does not match its EMA registration") + } else { + None + }; + if registration_error.is_some() && server.enabled { + server.enabled = false; + server.disabled_reason = Some(crate::McpServerDisabledReason::EmaRegistration); + } + server.auth = McpServerAuth::EmaAuth; + let oauth = server.oauth.get_or_insert_default(); + oauth.client_id = Some(self.client_id.clone()); + oauth.authorization_server_issuer = Some(self.authorization_server_issuer.clone()); + server.scopes = Some(self.scopes.clone()); + oauth.ema_registration = None; + oauth.ema_registration_error = registration_error; + server.oauth_resource = Some(self.resource.clone()); + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct MarketplaceConfig { diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 889174891a..4ba904023d 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -969,8 +969,8 @@ async fn load_plugin( fn apply_plugin_mcp_server_policy(config: &mut McpServerConfig, policy: &PluginMcpServerConfig) { config.enabled = policy.enabled; - if policy.ema_auth.is_some() { - config.auth = codex_config::McpServerAuth::EmaAuth; + if let Some(ema) = &policy.ema_auth { + ema.apply(config); } if let Some(approval_mode) = policy.default_tools_approval_mode { config.default_tools_approval_mode = Some(approval_mode); @@ -1438,11 +1438,11 @@ pub fn apply_configured_plugin_mcp_server_policies( ) { for (name, server) in servers { if let Some(policy) = policies.get(name) { - if policy.ema_auth.is_some() { - server.auth = codex_config::McpServerAuth::EmaAuth; + server.enabled &= policy.enabled; + if let Some(ema) = &policy.ema_auth { + ema.apply(server); } let declared_approval_mode = server.default_tools_approval_mode.unwrap_or_default(); - server.enabled &= policy.enabled; if let Some(approval_mode) = policy.default_tools_approval_mode { server.default_tools_approval_mode = diff --git a/codex-rs/core-plugins/src/loader_tests.rs b/codex-rs/core-plugins/src/loader_tests.rs index 34f75548ea..d3b6a247da 100644 --- a/codex-rs/core-plugins/src/loader_tests.rs +++ b/codex-rs/core-plugins/src/loader_tests.rs @@ -27,6 +27,102 @@ fn user_layer(path: AbsolutePathBuf, config: &str) -> ConfigLayerEntry { ) } +#[tokio::test] +async fn ema_policy_overlays_native_and_agent_plugins_without_changing_endpoints() { + let temp_dir = TempDir::new().expect("tempdir"); + let policy_config = r#" + [plugins."sample@test".mcp_servers.example] + enabled = true + [plugins."sample@test".mcp_servers.example.ema_auth] + url = "https://resource.example/mcp" + client_id = "resource-client" + authorization_server_issuer = "https://as.example" + resource = "https://resource.example" + scopes = ["tools"] + "#; + let stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::EnterpriseManaged { + id: "ema-policy".to_string(), + name: "EMA policy".to_string(), + }, + toml::from_str(policy_config).expect("managed policy toml"), + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("valid trusted policy stack"); + let policies = configured_plugins_from_stack(&stack, temp_dir.path()) + .remove("sample@test") + .expect("configured plugin") + .mcp_servers; + + for (name, manifest_path, manifest, mcp_path, mcp) in [ + ( + "native", + ".codex-plugin/plugin.json", + r#"{"name":"native"}"#, + ".mcp.json", + r#"{"mcpServers":{"example":{"url":"https://resource.example/mcp"}}}"#, + ), + ( + "agent", + "plugin.json", + r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/plugin.schema.json","name":"agent"}"#, + "mcp.json", + r#"{"$schema":"https://agent-plugins.org/schemas/1.0.0/mcp.schema.json","mcpServers":{"example":{"type":"streamable-http","url":"https://resource.example/mcp"}}}"#, + ), + ] { + for endpoint in [ + "https://resource.example/mcp", + "https://resource.example/mcp/other", + "https://resource.example/mcp?tenant=other", + "https://other.example/mcp", + ] { + let root = temp_dir.path().join(name); + write_file(&root.join(manifest_path), manifest); + write_file( + &root.join(mcp_path), + &mcp.replace("https://resource.example/mcp", endpoint), + ); + let declared = load_plugin_mcp_servers(&root, /*auth_mode*/ None).await; + let transport = declared["example"].transport.clone(); + let installed = load_plugin_mcp_servers_with_policy( + &root, + /*auth_mode*/ None, + Some(&policies), + ) + .await; + let mut selected = declared; + apply_configured_plugin_mcp_server_policies(&policies, &mut selected); + assert_eq!(installed, selected); + let server = &selected["example"]; + assert_eq!(server.transport, transport); + assert_eq!(server.enabled, endpoint == "https://resource.example/mcp"); + assert_eq!(server.auth, codex_config::McpServerAuth::EmaAuth); + assert_eq!( + server.oauth, + Some(codex_config::McpServerOAuthConfig { + client_id: Some("resource-client".into()), + authorization_server_issuer: Some("https://as.example".into()), + ema_registration_error: (endpoint != "https://resource.example/mcp") + .then_some("plugin endpoint does not match its EMA registration"), + ..Default::default() + }) + ); + assert_eq!( + server.oauth_resource.as_deref(), + Some("https://resource.example") + ); + assert_eq!( + server.scopes.as_deref(), + Some(["tools".to_string()].as_slice()) + ); + assert_eq!(server.oauth_idp(), None); + } + } +} + #[tokio::test] async fn agent_plugin_overlay_apps_are_not_runtime_active() { let temp_dir = TempDir::new().expect("tempdir"); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 96c2a43f22..0e36b5fd1a 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -835,6 +835,9 @@ pub struct Config { /// Definition for MCP servers that Codex can reach out to for tool calls. pub mcp_servers: Constrained>, + /// Trusted IdP shared by all permitted EMA MCP registrations. + pub mcp_enterprise_managed_auth: Option, + /// When present, only these MCP servers omit the legacy `mcp__` namespace prefix. pub non_prefixed_mcp_tool_servers: Option>, @@ -1706,6 +1709,11 @@ impl Config { additional_plugin_registrations: impl IntoIterator, ) -> McpConfig { let mut catalog = ResolvedMcpCatalog::builder(); + if self.features.enabled(Feature::UseXaa) + && let Some(auth) = &self.mcp_enterprise_managed_auth + { + catalog.enable_ema(auth.idp.clone()); + } for (plugin_order, plugin) in loaded_plugins .plugins() .iter() @@ -1749,6 +1757,9 @@ impl Config { chatgpt_base_url: self.chatgpt_base_url.clone(), apps_mcp_product_sku: self.apps_mcp_product_sku.clone(), codex_home: self.codex_home.to_path_buf(), + mcp_enterprise_managed_auth: self.mcp_enterprise_managed_auth.clone(), + xaa_enabled: self.features.enabled(Feature::UseXaa) + && self.mcp_enterprise_managed_auth.is_some(), mcp_oauth_credentials_store_mode: self.mcp_oauth_credentials_store_mode, oauth_refresh_mode: if self.features.enabled(Feature::McpOAuthRefreshCoordination) { McpOAuthRefreshMode::Coordinated @@ -1822,33 +1833,8 @@ impl Config { &self, refreshed_config: &Config, ) -> std::io::Result { - let mut layers = refreshed_config - .config_layer_stack - .all_layers_low_to_high() - .filter(|layer| !is_session_layer(&layer.name)) - .cloned() - .collect::>(); - layers.extend( - self.config_layer_stack - .all_layers_low_to_high() - .filter(|layer| is_session_layer(&layer.name)) - .cloned(), - ); - layers.sort_by_key(|layer| layer.name.precedence()); - - let config_layer_stack = ConfigLayerStack::new( - layers, - refreshed_config.config_layer_stack.requirements().clone(), - refreshed_config - .config_layer_stack - .requirements_toml() - .clone(), - )? - .with_user_and_project_exec_policy_rules_ignored( - refreshed_config - .config_layer_stack - .ignore_user_and_project_exec_policy_rules(), - ); + let config_layer_stack = + self.layer_stack_preserving_session(&refreshed_config.config_layer_stack)?; let cfg: ConfigToml = config_layer_stack .effective_config() .try_into() @@ -1873,6 +1859,33 @@ impl Config { .await } + fn layer_stack_preserving_session( + &self, + refreshed_layers: &ConfigLayerStack, + ) -> std::io::Result { + let mut layers = refreshed_layers + .all_layers_low_to_high() + .filter(|layer| !is_session_layer(&layer.name)) + .cloned() + .collect::>(); + layers.extend( + self.config_layer_stack + .all_layers_low_to_high() + .filter(|layer| is_session_layer(&layer.name)) + .cloned(), + ); + layers.sort_by_key(|layer| layer.name.precedence()); + + Ok(ConfigLayerStack::new( + layers, + refreshed_layers.requirements().clone(), + refreshed_layers.requirements_toml().clone(), + )? + .with_user_and_project_exec_policy_rules_ignored( + refreshed_layers.ignore_user_and_project_exec_policy_rules(), + )) + } + /// This is the preferred way to create an instance of [Config]. pub async fn load_with_cli_overrides( cli_overrides: Vec<(String, TomlValue)>, @@ -3291,7 +3304,7 @@ impl Config { feature_requirements, &mut startup_warnings, )?; - let _ = McpEnterpriseManagedAuthConfig::resolve( + let mcp_enterprise_managed_auth = McpEnterpriseManagedAuthConfig::resolve( &config_layer_stack, cfg.mcp_enterprise_managed_auth.as_ref(), &cfg.mcp_servers, @@ -4170,6 +4183,7 @@ impl Config { }, mcp_servers, non_prefixed_mcp_tool_servers, + mcp_enterprise_managed_auth, // The config.toml omits "_mode" because it's a config file. However, "_mode" // is important in code to differentiate the mode from the store implementation. mcp_oauth_credentials_store_mode: resolve_mcp_oauth_credentials_store_mode( diff --git a/codex-rs/core/tests/suite/mcp_ema_config.rs b/codex-rs/core/tests/suite/mcp_ema_config.rs index 1ebdcff5c1..08324fadae 100644 --- a/codex-rs/core/tests/suite/mcp_ema_config.rs +++ b/codex-rs/core/tests/suite/mcp_ema_config.rs @@ -1,4 +1,6 @@ -//! Exercises EMA registration ownership through real configuration layers. +//! Exercises EMA activation and registration ownership through real configuration layers. + +use std::sync::Arc; use anyhow::Result; use codex_config::LoaderOverrides; @@ -6,10 +8,117 @@ use codex_config::McpServerAuth; use codex_config::test_support::CloudConfigBundleFixture; use codex_core::config::ConfigBuilder; use codex_core::config::set_project_trust_level; +use codex_login::CodexAuth; use codex_protocol::config_types::TrustLevel; +use codex_protocol::protocol::EventMsg; +use core_test_support::responses; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event_match; use pretty_assertions::assert_eq; +use serde_json::json; use tempfile::tempdir; use test_case::test_case; +use wiremock::MockServer; + +#[derive(Clone, Copy)] +enum ActivationScenario { + FeatureDisabled, + MissingIdp, + PluginSelfOptIn, + OperatorEnabled, +} + +#[test_case(ActivationScenario::FeatureDisabled; "feature disabled")] +#[test_case(ActivationScenario::MissingIdp; "missing IdP")] +#[test_case(ActivationScenario::PluginSelfOptIn; "plugin cannot select enterprise auth")] +#[test_case(ActivationScenario::OperatorEnabled; "operator registration is eligible for startup")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn enterprise_activation_respects_managed_config_and_plugin_ownership( + scenario: ActivationScenario, +) -> Result<()> { + skip_if_no_network!(Ok(())); + let server = MockServer::start().await; + let responses_server = responses::start_mock_server().await; + let home = Arc::new(tempdir()?); + let resource = format!("{}/mcp", server.uri()); + let issuer = format!("{}/idp", server.uri()); + let xaa_enabled = !matches!(scenario, ActivationScenario::FeatureDisabled); + let mut managed_config = format!( + "mcp_oauth_credentials_store = \"file\"\n[features]\nuse_xaa = {xaa_enabled}\nsecret_auth_storage = false\napps = false\n" + ); + if !matches!(scenario, ActivationScenario::MissingIdp) { + managed_config.push_str(&format!( + "[mcp_enterprise_managed_auth.idp]\nissuer = {issuer:?}\nclient_id = \"idp-client\"\n" + )); + } + if matches!(scenario, ActivationScenario::PluginSelfOptIn) { + let plugin_root = super::plugins::write_sample_plugin_manifest_and_config(&home); + std::fs::write( + plugin_root.join(".mcp.json"), + serde_json::to_vec(&json!({"mcpServers": {"enterprise": { + "url": resource, "auth": "ema_auth", "oauth": {"client_id": "mcp-client"} + }}}))?, + )?; + } else { + managed_config.push_str(&format!( + "[mcp_servers.enterprise]\nurl = {resource:?}\nauth = \"ema_auth\"\n[mcp_servers.enterprise.oauth]\nclient_id = \"mcp-client\"\n" + )); + } + let fixture = test_codex() + .with_home(home) + .with_auth(CodexAuth::from_api_key("test-api-key")) + .with_cloud_config_bundle(CloudConfigBundleFixture::loader_with_enterprise_config( + managed_config, + )) + .build_with_auto_env(&responses_server) + .await?; + let (runtime_config, _) = fixture.codex.current_mcp_config_and_runtime_context().await; + let expected_enabled = match scenario { + ActivationScenario::FeatureDisabled | ActivationScenario::MissingIdp => Some(false), + ActivationScenario::PluginSelfOptIn => None, + ActivationScenario::OperatorEnabled => Some(true), + }; + assert_eq!( + codex_mcp::configured_mcp_servers(&runtime_config) + .get("enterprise") + .map(|server| server.enabled), + expected_enabled, + ); + let startup = wait_for_event_match(&fixture.codex, |event| match event { + EventMsg::McpStartupComplete(summary) => Some(summary.clone()), + _ => None, + }) + .await; + // Eligible registration reaches startup, which fails closed without usable EMA auth. + // Ineligible declarations must never start, even when their URLs are reachable. + let expected_failed = if expected_enabled == Some(true) { + vec!["enterprise".to_string()] + } else { + Vec::new() + }; + assert_eq!( + ( + startup.ready, + startup + .failed + .into_iter() + .map(|failure| failure.server) + .collect::>(), + startup.cancelled, + ), + (Vec::::new(), expected_failed, Vec::::new()), + ); + fixture.codex.shutdown_and_wait().await?; + assert!( + server + .received_requests() + .await + .expect("recorded requests") + .is_empty() + ); + Ok(()) +} #[test_case("oauth"; "OAuth")] #[test_case("chatgpt"; "ChatGPT")] diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 47ab3e0926..dfe19337af 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -249,6 +249,7 @@ fn new_config(model: Option, arg0_paths: Arg0DispatchPaths) -> anyhow::R workspace_roots_explicit: false, cli_auth_credentials_store_mode: AuthCredentialsStoreMode::File, mcp_servers: Constrained::allow_any(HashMap::new()), + mcp_enterprise_managed_auth: None, non_prefixed_mcp_tool_servers: None, mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::File, mcp_oauth_callback_port: None,