From e5253b97fb6c39b984d84acb409d0e9552de407f Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Mon, 15 Jun 2026 14:04:01 +0100 Subject: [PATCH] [codex] Skip plugin MCP OAuth for matching app routes (#27461) ## Context This is PR5 in the plugin auth-routing stack. Earlier PRs make plugin surface projection auth-aware, narrow App/MCP conflicts by App declaration name, and keep connector listings auth-aware. This PR applies the same name-based App/MCP conflict rule into plugin MCP loading, so install-time MCP OAuth and plugin detail metadata both reflect the MCPs available for the current auth route. ## Stack - PR1: #27652 seed plugin manager auth at construction. - PR2: #27459 route plugin surfaces by auth mode. - PR3: #27607 dedupe plugin MCP servers by App declaration name. - PR4: #27602 preserve plugin Apps in connector listings. - PR5: #27461 skip install-time plugin MCP OAuth for matching App routes. ## Summary - Make `load_plugin_mcp_servers` auth-aware and let it load App declarations before filtering same-name MCP servers for Codex-backend auth. - Use that filtered MCP list for both install-time MCP OAuth and marketplace plugin detail metadata. - Preserve API-key/direct auth behavior so plugin MCP servers remain visible and can still start OAuth. ## Validation ```bash cargo fmt --all cargo test -p codex-core-plugins read_plugin_for_config_filters_mcp_servers_for_codex_backend_auth cargo check -p codex-core-plugins -p codex-app-server git diff --check git diff --cached --check ``` --- .../src/request_processors/plugins.rs | 13 +- .../tests/suite/v2/plugin_install.rs | 378 ++++++++++++++++++ codex-rs/core-plugins/src/loader.rs | 25 +- codex-rs/core-plugins/src/manager.rs | 2 +- codex-rs/core-plugins/src/manager_tests.rs | 76 ++++ 5 files changed, 489 insertions(+), 5 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 80f5c2aad..449689871 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -1436,7 +1436,11 @@ impl PluginRequestProcessor { self.on_effective_plugins_changed(); - let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await; + let plugin_mcp_servers = load_plugin_mcp_servers( + result.installed_path.as_path(), + auth.as_ref().map(CodexAuth::auth_mode), + ) + .await; if !plugin_mcp_servers.is_empty() { self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) .await; @@ -1445,7 +1449,6 @@ impl PluginRequestProcessor { let plugin_app_declarations = load_plugin_apps(result.installed_path.as_path()).await; let plugin_apps = codex_plugin::app_connector_ids_from_declarations(&plugin_app_declarations); - let auth = self.auth_manager.auth().await; let apps_needing_auth = self .plugin_apps_needing_auth_for_install( &config, @@ -1554,7 +1557,11 @@ impl PluginRequestProcessor { self.analytics_events_client .track_plugin_installed(plugin_metadata); - let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await; + let plugin_mcp_servers = load_plugin_mcp_servers( + result.installed_path.as_path(), + auth.as_ref().map(CodexAuth::auth_mode), + ) + .await; if !plugin_mcp_servers.is_empty() { self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers) .await; 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 1be64b8fa..6965e759e 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -1027,6 +1027,293 @@ async fn plugin_install_returns_apps_needing_auth() -> Result<()> { Ok(()) } +#[tokio::test] +async fn plugin_install_skips_mcp_oauth_for_chatgpt_dual_surface_plugin() -> Result<()> { + let connectors = vec![AppInfo { + id: "sample-mcp".to_string(), + name: "Sample MCP".to_string(), + description: Some("Sample MCP connector".to_string()), + logo_url: Some("https://example.com/alpha.png".to_string()), + logo_url_dark: None, + distribution_channel: Some("featured".to_string()), + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: false, + is_enabled: true, + plugin_display_names: Vec::new(), + }]; + let (apps_server_url, apps_server_handle, _apps_server_control) = + start_apps_server(connectors, Vec::new()).await?; + let oauth_server = MockServer::start().await; + + let codex_home = TempDir::new()?; + write_connectors_config(codex_home.path(), &apps_server_url)?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + /*install_policy*/ None, + /*auth_policy*/ None, + )?; + write_plugin_source(repo_root.path(), "sample-plugin", &["sample-mcp"])?; + write_plugin_mcp_config(repo_root.path(), "sample-plugin", &oauth_server.uri())?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstallResponse = to_response(response)?; + + assert_eq!(response.auth_policy, PluginAuthPolicy::OnInstall); + assert_eq!(oauth_discovery_request_count(&oauth_server).await, 0); + + apps_server_handle.abort(); + let _ = apps_server_handle.await; + Ok(()) +} + +#[tokio::test] +async fn plugin_install_starts_mcp_oauth_when_only_plugin_apps_are_disallowed() -> Result<()> { + let (apps_server_url, apps_server_handle, _apps_server_control) = + start_apps_server(Vec::new(), Vec::new()).await?; + let oauth_server = MockServer::start().await; + + let codex_home = TempDir::new()?; + write_connectors_config(codex_home.path(), &apps_server_url)?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("chatgpt-token") + .account_id("account-123") + .chatgpt_user_id("user-123") + .chatgpt_account_id("account-123"), + AuthCredentialsStoreMode::File, + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + /*install_policy*/ None, + /*auth_policy*/ None, + )?; + write_plugin_source( + repo_root.path(), + "sample-plugin", + &["asdk_app_6938a94a61d881918ef32cb999ff937c"], + )?; + write_plugin_mcp_config(repo_root.path(), "sample-plugin", &oauth_server.uri())?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstallResponse = to_response(response)?; + + assert_eq!(response.auth_policy, PluginAuthPolicy::OnInstall); + assert_eq!(response.apps_needing_auth, Vec::::new()); + assert!(oauth_discovery_request_count(&oauth_server).await > 0); + + apps_server_handle.abort(); + let _ = apps_server_handle.await; + Ok(()) +} + +#[tokio::test] +async fn plugin_install_starts_mcp_oauth_for_api_key_dual_surface_plugin() -> Result<()> { + let oauth_server = MockServer::start().await; + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#" +mcp_oauth_credentials_store = "file" + +[features] +plugins = true +connectors = true +"#, + )?; + + let repo_root = TempDir::new()?; + write_plugin_marketplace( + repo_root.path(), + "debug", + "sample-plugin", + "./sample-plugin", + /*install_policy*/ None, + /*auth_policy*/ None, + )?; + write_plugin_source(repo_root.path(), "sample-plugin", &["sample-mcp"])?; + write_plugin_mcp_config(repo_root.path(), "sample-plugin", &oauth_server.uri())?; + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.path().join(".agents/plugins/marketplace.json"))?; + + let mut mcp = TestAppServer::new_with_env( + codex_home.path(), + &[("OPENAI_API_KEY", Some("test-api-key"))], + ) + .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_plugin_install_request(PluginInstallParams { + marketplace_path: Some(marketplace_path), + remote_marketplace_name: None, + plugin_name: "sample-plugin".to_string(), + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstallResponse = to_response(response)?; + + assert_eq!(response.auth_policy, PluginAuthPolicy::OnInstall); + assert!(oauth_discovery_request_count(&oauth_server).await > 0); + Ok(()) +} + +#[tokio::test] +async fn plugin_install_starts_remote_mcp_oauth_for_install_response_only_app() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + let oauth_server = MockServer::start().await; + let bundle_url = mount_remote_plugin_bundle( + &server, + /*status_code*/ 200, + remote_plugin_bundle_tar_gz_bytes_with_mcp_config("linear", &oauth_server.uri())?, + ) + .await; + configure_remote_plugin_with_apps_test(codex_home.path(), &server)?; + mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await; + mount_empty_remote_installed_plugins(&server).await; + mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await; + + let mut mcp = TestAppServer::new_with_env( + codex_home.path(), + &[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))], + ) + .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = send_remote_plugin_install_request(&mut mcp, REMOTE_PLUGIN_ID).await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstallResponse = to_response(response)?; + + assert_eq!( + response, + PluginInstallResponse { + auth_policy: PluginAuthPolicy::OnUse, + apps_needing_auth: vec![AppSummary { + id: "alpha".to_string(), + name: "alpha".to_string(), + description: None, + install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), + category: None, + }], + } + ); + assert!(oauth_discovery_request_count(&oauth_server).await > 0); + Ok(()) +} + +#[tokio::test] +async fn plugin_install_skips_remote_mcp_oauth_for_bundled_same_name_app() -> Result<()> { + let codex_home = TempDir::new()?; + let server = MockServer::start().await; + let oauth_server = MockServer::start().await; + let bundle_url = mount_remote_plugin_bundle( + &server, + /*status_code*/ 200, + remote_plugin_bundle_tar_gz_bytes_with_app_and_mcp_config( + "linear", + r#"{"apps":{"sample-mcp":{"id":"alpha"}}}"#, + &oauth_server.uri(), + )?, + ) + .await; + configure_remote_plugin_with_apps_test(codex_home.path(), &server)?; + mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await; + mount_empty_remote_installed_plugins(&server).await; + mount_remote_plugin_install_with_apps_needing_auth(&server, REMOTE_PLUGIN_ID, &["alpha"]).await; + + let mut mcp = TestAppServer::new_with_env( + codex_home.path(), + &[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))], + ) + .await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = send_remote_plugin_install_request(&mut mcp, REMOTE_PLUGIN_ID).await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: PluginInstallResponse = to_response(response)?; + + assert_eq!( + response, + PluginInstallResponse { + auth_policy: PluginAuthPolicy::OnUse, + apps_needing_auth: vec![AppSummary { + id: "alpha".to_string(), + name: "alpha".to_string(), + description: None, + install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), + category: None, + }], + } + ); + assert_eq!(oauth_discovery_request_count(&oauth_server).await, 0); + Ok(()) +} + #[tokio::test] async fn plugin_install_filters_disallowed_apps_needing_auth() -> Result<()> { let connectors = vec![AppInfo { @@ -1436,6 +1723,16 @@ async fn wait_for_plugin_analytics_payload(server: &MockServer) -> Result usize { + server + .received_requests() + .await + .unwrap_or_default() + .iter() + .filter(|request| request.url.path().contains("oauth-authorization-server")) + .count() +} + fn write_remote_plugin_catalog_config( codex_home: &std::path::Path, base_url: &str, @@ -1814,14 +2111,92 @@ fn write_plugin_source( Ok(()) } +fn write_plugin_mcp_config( + repo_root: &std::path::Path, + plugin_name: &str, + mcp_base_url: &str, +) -> Result<()> { + std::fs::write( + repo_root.join(plugin_name).join(".mcp.json"), + format!( + r#"{{ + "mcpServers": {{ + "sample-mcp": {{ + "type": "http", + "url": "{mcp_base_url}/mcp" + }} + }} +}}"# + ), + )?; + Ok(()) +} + fn remote_plugin_bundle_tar_gz_bytes(plugin_name: &str) -> Result> { let manifest = format!(r#"{{"name":"{plugin_name}"}}"#); remote_plugin_bundle_tar_gz_bytes_with_contents(&manifest, /*app_manifest*/ None) } +fn remote_plugin_bundle_tar_gz_bytes_with_mcp_config( + plugin_name: &str, + mcp_base_url: &str, +) -> Result> { + let manifest = format!(r#"{{"name":"{plugin_name}"}}"#); + let mcp_config = format!( + r#"{{ + "mcpServers": {{ + "sample-mcp": {{ + "type": "http", + "url": "{mcp_base_url}/mcp" + }} + }} +}}"# + ); + remote_plugin_bundle_tar_gz_bytes_with_entries( + &manifest, + /*app_manifest*/ None, + Some(mcp_config.as_str()), + ) +} + +fn remote_plugin_bundle_tar_gz_bytes_with_app_and_mcp_config( + plugin_name: &str, + app_manifest: &str, + mcp_base_url: &str, +) -> Result> { + let manifest = format!(r#"{{"name":"{plugin_name}"}}"#); + let mcp_config = format!( + r#"{{ + "mcpServers": {{ + "sample-mcp": {{ + "type": "http", + "url": "{mcp_base_url}/mcp" + }} + }} +}}"# + ); + remote_plugin_bundle_tar_gz_bytes_with_entries( + &manifest, + Some(app_manifest), + Some(mcp_config.as_str()), + ) +} + fn remote_plugin_bundle_tar_gz_bytes_with_contents( plugin_manifest: &str, app_manifest: Option<&str>, +) -> Result> { + remote_plugin_bundle_tar_gz_bytes_with_entries( + plugin_manifest, + app_manifest, + /*mcp_config*/ None, + ) +} + +fn remote_plugin_bundle_tar_gz_bytes_with_entries( + plugin_manifest: &str, + app_manifest: Option<&str>, + mcp_config: Option<&str>, ) -> Result> { let skill = "# Plan Work\n\nTrack work in Linear.\n"; let encoder = GzEncoder::new(Vec::new(), Compression::default()); @@ -1841,6 +2216,9 @@ fn remote_plugin_bundle_tar_gz_bytes_with_contents( if let Some(app_manifest) = app_manifest { entries.push((".app.json", app_manifest.as_bytes(), /*mode*/ 0o644)); } + if let Some(mcp_config) = mcp_config { + entries.push((".mcp.json", mcp_config.as_bytes(), /*mode*/ 0o644)); + } for (path, contents, mode) in entries { let mut header = tar::Header::new_gnu(); header.set_size(contents.len() as u64); diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 146908f88..ad7654765 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -10,6 +10,7 @@ use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RemoteInstalledPlugin; use crate::store::PluginStore; use crate::store::plugin_version_for_source; +use codex_app_server_protocol::AuthMode; use codex_config::ConfigLayerStack; use codex_config::HooksFile; use codex_config::types::McpServerConfig; @@ -1079,7 +1080,29 @@ pub async fn plugin_telemetry_metadata_from_root( } } -pub async fn load_plugin_mcp_servers(plugin_root: &Path) -> HashMap { +pub async fn load_plugin_mcp_servers( + plugin_root: &Path, + auth_mode: Option, +) -> HashMap { + let mut mcp_servers = load_declared_plugin_mcp_servers(plugin_root).await; + if !auth_mode.is_some_and(AuthMode::uses_codex_backend) || mcp_servers.is_empty() { + return mcp_servers; + } + + let app_declarations = load_plugin_apps(plugin_root).await; + if app_declarations.is_empty() { + return mcp_servers; + } + + let app_declaration_names = app_declarations + .iter() + .map(|app| app.name.as_str()) + .collect::>(); + mcp_servers.retain(|name, _| !app_declaration_names.contains(name.as_str())); + mcp_servers +} + +async fn load_declared_plugin_mcp_servers(plugin_root: &Path) -> HashMap { let Some(manifest) = load_plugin_manifest(plugin_root) else { return HashMap::new(); }; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 7ac0fd070..b973dc729 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -1330,7 +1330,7 @@ impl PluginsManager { app_category_by_id.insert(app.connector_id.0.clone(), category.clone()); } } - let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path()) + let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path(), self.auth_mode()) .await .into_keys() .collect::>(); diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 78f7d3e08..2e68c8b25 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -2425,6 +2425,82 @@ enabled = true assert!(matches!(err, MarketplaceError::PluginsDisabled)); } +#[tokio::test] +async fn read_plugin_for_config_filters_mcp_servers_for_codex_backend_auth() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + write_file( + &repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample-plugin", + "source": { + "source": "local", + "path": "./sample-plugin" + } + } + ] +}"#, + ); + write_file( + &repo_root.join("sample-plugin/.codex-plugin/plugin.json"), + r#"{"name":"sample-plugin"}"#, + ); + write_file( + &repo_root.join("sample-plugin/.app.json"), + r#"{"apps":{"sample-mcp":{"id":"connector_sample"}}}"#, + ); + write_file( + &repo_root.join("sample-plugin/.mcp.json"), + r#"{"mcpServers":{"other-mcp":{"command":"other-mcp"},"sample-mcp":{"command":"sample-mcp"}}}"#, + ); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + + let config = load_config(tmp.path(), &repo_root).await; + let request = PluginReadRequest { + plugin_name: "sample-plugin".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + repo_root.join(".agents/plugins/marketplace.json"), + ) + .unwrap(), + }; + + let chatgpt_outcome = PluginsManager::new_with_options( + tmp.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ) + .read_plugin_for_config(&config, &request) + .await + .unwrap(); + assert_eq!( + chatgpt_outcome.plugin.mcp_server_names, + vec!["other-mcp".to_string()] + ); + + let api_key_outcome = PluginsManager::new_with_options( + tmp.path().to_path_buf(), + Some(Product::Codex), + Some(AuthMode::ApiKey), + ) + .read_plugin_for_config(&config, &request) + .await + .unwrap(); + assert_eq!( + api_key_outcome.plugin.mcp_server_names, + vec!["other-mcp".to_string(), "sample-mcp".to_string()] + ); +} + #[tokio::test] async fn read_plugin_for_config_uses_user_layer_skill_settings_only() { let tmp = tempfile::tempdir().unwrap();